diff --git a/docs/requirements/Component-Transport.md b/docs/requirements/Component-Transport.md index 9345cdf9..579a40b4 100644 --- a/docs/requirements/Component-Transport.md +++ b/docs/requirements/Component-Transport.md @@ -301,6 +301,8 @@ Site identifiers and connection names are environment-specific, so a bundle expo **Auto-match → operator override → apply.** The importer first auto-matches every source site and connection against the target by these identity keys. The operator confirms/overrides via the Central UI **Map step** (Step 3) or the CLI `--map-site` / `--map-connection` / `--create-missing-*` flags. References that neither auto-match nor are set to Create-new become **blocker rows** (UI) — or, on the CLI without `--create-missing-sites` / `--create-missing-connections`, abort before apply. +**Template library reads (review 2026-10-21 04/P4 + 08/P1).** A preview and each apply attempt read the target template library **once**, through body-free projections (`GetTemplateStubsAsync` for id/name maps, `GetAllTemplatesForAnalysisAsync` — untracked, members and lock flags but no script bodies — where members or edges are needed), held on a per-session `TemplateLibrary` and threaded to every consumer: preview diff names, blocker and locked-override scans, apply-time validation, the composition/inheritance rewire fallbacks (formerly a full library reload *inside* the per-template loop), the instance pass's name→id map (overlaid with the change tracker's just-flushed templates), the acyclicity guard and the stale-instance expansion. The full tracked graph is loaded only for the templates the bundle **Overwrites** (`GetTemplatesWithChildrenAsync` by name), since their child syncs mutate it. The acyclicity guard reads the change tracker's staged templates overlaid on that snapshot — the untouched part of the graph has not changed during the apply. Instance lookups are batched by name set (`ITemplateEngineRepository.GetInstancesByUniqueNamesAsync`, one tracked + hydrated read for preview and one for the instance pass, Rename targets included), and the preview resolves an existing instance's template / site / area names from the library, one site list and one area list per site, and reads shared scripts and external systems once each. + **Template second-pass rewires.** Bundles carry template-to-template references by **name**, not id (ids are environment-specific). After the templates are flushed, the importer runs three name-resolution passes over the imported templates so every cross-template FK points at the freshly-persisted target row: the **alarm on-trigger-script** links (`TemplateAlarm.OnTriggerScriptId`), the **composition** edges (`TemplateComposition`), and the **inheritance** edges (`Template.ParentTemplateId`, resolved from `BaseTemplateName` through the same rename/skip resolution map — a `null` base clears a stale edge on Overwrite). Each pass is best-effort: an unresolvable reference leaves the FK null and emits a warning audit row (`BundleImportAlarmScriptUnresolved` / `BundleImportCompositionUnresolved` / `BundleImportBaseTemplateUnresolved`) rather than aborting the import. **Rename call-site limitation (load-bearing).** A `Rename` resolution renames the entity being imported (e.g. a SharedScript `Foo` → `Bar`), but it does **not** rewrite call sites inside importing scripts — a script body that calls `CallShared("Foo")` still says `Foo` after import. So the target DB can end up with scripts referencing the old name. This is deliberate: Pass 1 name resolution registers **both** the original and renamed names as resolvable (so the import isn't blocked), and the authoritative check is the target's own **deploy-time validation** (`ScriptCompiler.TryCompile` in the Template Engine deploy gate), which recompiles against the actual persisted catalog. Operators renaming a shared dependency must fix the calling scripts on the target (or re-export with the dependency included under its original name). @@ -314,7 +316,7 @@ Site identifiers and connection names are environment-specific, so a bundle expo There is no explicit stale-mark write. Overwriting a template during import changes its content, which naturally changes the flattened-config hash that `DeploymentService.CompareAsync` computes at query time. The Deployments page already surfaces any instance whose computed hash differs from the deployed snapshot — no new mechanism is required. -As of M8 the import result enumerates affected instances for real (closing the prior `StaleInstanceIds` stub). Before commit, for each **Overwritten** template the importer walks the existing **deployed** target instances of that template and, via the `IStaleInstanceProbe` seam in Commons (implemented in the Deployment Manager over the flattening pipeline), compares each instance's freshly-flattened revision hash against its `DeployedConfigSnapshot.RevisionHash`; instances whose hash has drifted are returned in `ImportResult.StaleInstanceIds`. The probe is optional — when it is not registered the result is an empty set (informational only, never blocking). Freshly-imported `NotDeployed` instances are surfaced as **new**, not stale. Step 5 (Confirm) shows the real count and Step 6 (Result) deep-links the Deployments page filtered to those instances. +As of M8 the import result enumerates affected instances for real (closing the prior `StaleInstanceIds` stub). Before commit the importer takes the set of **Overwritten** templates and — review 2026-10-21 04/C2 — expands it over the post-apply template graph to every template whose flattened config includes one of them: **derived templates** (via `ParentTemplateId`) and **templates that compose it** (via `TemplateComposition.ComposedTemplateId`), transitively. Overwriting a base or a feature module with no direct instances therefore still reports the deployed instances of everything built on it (previously only instances whose `TemplateId` was the overwritten id were probed, so such an import reported 0 stale instances and the operator was told nothing needed redeploying). It then walks the **deployed** target instances of every affected template and, via the `IStaleInstanceProbe` seam in Commons (implemented in the Deployment Manager over the flattening pipeline), compares each instance's freshly-flattened revision hash against its latest `DeployedConfigSnapshot.RevisionHash` (read for all candidates in one projected query); instances whose hash has drifted are returned in `ImportResult.StaleInstanceIds`. The probe is used through `IStaleInstanceProbe.BeginBatch()`, which shares one `FlattenSession` across the whole check so a template chain common to many instances is walked once. The probe is optional — when it is not registered the result is an empty set (informational only, never blocking). Freshly-imported `NotDeployed` instances are surfaced as **new**, not stale. Step 5 (Confirm) shows the real count and Step 6 (Result) deep-links the Deployments page filtered to those instances. ## Error Handling diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ITemplateEngineRepository.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ITemplateEngineRepository.cs index 0a2df109..5cdc8c15 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ITemplateEngineRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ITemplateEngineRepository.cs @@ -262,6 +262,15 @@ public interface ITemplateEngineRepository /// Cancellation token. /// A task that resolves to the matching instance, or if not found. Task GetInstanceByUniqueNameAsync(string uniqueName, CancellationToken cancellationToken = default); + /// + /// Retrieves every instance whose unique name is in , with the + /// same four child collections includes, in one + /// round trip (tracked). Unknown names are simply absent from the result. + /// + /// The unique instance names to look up; null/empty/duplicate entries are ignored. + /// Cancellation token. + /// A task that resolves to the matching instances. + Task> GetInstancesByUniqueNamesAsync(IEnumerable uniqueNames, CancellationToken cancellationToken = default); /// Adds a new instance. /// The instance to add. /// Cancellation token. diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Transport/IStaleInstanceProbe.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Transport/IStaleInstanceProbe.cs index 0bcdadb7..76885599 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Transport/IStaleInstanceProbe.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Transport/IStaleInstanceProbe.cs @@ -29,4 +29,14 @@ public interface IStaleInstanceProbe /// Cancellation token. /// The current revision hash (e.g. sha256:...), or null. Task GetCurrentRevisionHashAsync(int instanceId, CancellationToken cancellationToken = default); + + /// + /// Returns a probe for hashing a batch of instances that may share template + /// chains — the implementation can reuse one flatten session across the batch so + /// a chain is walked once, not once per instance (review 2026-10-21 04/C2). Use + /// the returned probe for one batch only, within the caller's scope. The default + /// is this probe itself (no sharing). + /// + /// A probe to use for the batch. + IStaleInstanceProbe BeginBatch() => this; } diff --git a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/TemplateEngineRepository.cs b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/TemplateEngineRepository.cs index 5ed6e36d..9d378d33 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/TemplateEngineRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/TemplateEngineRepository.cs @@ -476,6 +476,30 @@ public class TemplateEngineRepository : ITemplateEngineRepository .FirstOrDefaultAsync(i => i.UniqueName == uniqueName, cancellationToken); } + /// + public async Task> GetInstancesByUniqueNamesAsync( + IEnumerable uniqueNames, CancellationToken cancellationToken = default) + { + // Bulk variant for the bundle importer's preview + instance pass, which used to + // call GetInstanceByUniqueNameAsync once per bundled instance (review 2026-10-21 + // 04/P4). De-duplicate before the query so EF emits a clean IN list. + if (uniqueNames is null) return Array.Empty(); + var distinct = uniqueNames + .Where(n => !string.IsNullOrEmpty(n)) + .Distinct(StringComparer.Ordinal) + .ToArray(); + if (distinct.Length == 0) return Array.Empty(); + + return await _context.Instances + .Where(i => distinct.Contains(i.UniqueName)) + .Include(i => i.AttributeOverrides) + .Include(i => i.AlarmOverrides) + .Include(i => i.ConnectionBindings) + .Include(i => i.NativeAlarmSourceOverrides) + .AsSplitQuery() + .ToListAsync(cancellationToken); + } + /// public async Task AddInstanceAsync(Instance instance, CancellationToken cancellationToken = default) { diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/StaleInstanceProbe.cs b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/StaleInstanceProbe.cs index 1dc2ec67..dc906c77 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/StaleInstanceProbe.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/StaleInstanceProbe.cs @@ -24,4 +24,33 @@ public sealed class StaleInstanceProbe : IStaleInstanceProbe instanceId, _flatteningPipeline.CreateSession(), cancellationToken).ConfigureAwait(false); return result.IsSuccess ? result.Value : null; } + + /// + /// + /// The batch probe shares ONE across its calls, so an + /// import that stale-checks many instances of the same (or related) templates walks + /// each template chain once (review 2026-10-21 04/C2). The session caches only what + /// it read during this batch, so it is no staler than the per-call session. + /// + public IStaleInstanceProbe BeginBatch() => new BatchProbe(_flatteningPipeline, _flatteningPipeline.CreateSession()); + + private sealed class BatchProbe : IStaleInstanceProbe + { + private readonly IFlatteningPipeline _pipeline; + private readonly FlattenSession _session; + + public BatchProbe(IFlatteningPipeline pipeline, FlattenSession session) + { + _pipeline = pipeline; + _session = session; + } + + public async Task GetCurrentRevisionHashAsync(int instanceId, CancellationToken cancellationToken = default) + { + var result = await _pipeline.ComputeRevisionHashAsync(instanceId, _session, cancellationToken).ConfigureAwait(false); + return result.IsSuccess ? result.Value : null; + } + + public IStaleInstanceProbe BeginBatch() => this; + } } diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 126f08b7..5531bd49 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -22,6 +22,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Messages.Scripting; using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Templates; using ZB.MOM.WW.ScadaBridge.Commons.Validation; using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase; using ZB.MOM.WW.ScadaBridge.ScriptAnalysis; @@ -450,6 +451,10 @@ public sealed class BundleImporter : IBundleImporter var content = ReadSessionContent(session); + // Review 2026-10-21 04/P4 + 08/P1: the template library is read once per + // preview through body-free projections, never as the full tracked graph. + var library = new TemplateLibrary(_templateRepo); + var items = new List(); // ---- TemplateFolders ---- @@ -473,8 +478,7 @@ public sealed class BundleImporter : IBundleImporter .ConfigureAwait(false); var hydratedByName = hydratedTemplates .ToDictionary(t => t.Name, t => t, StringComparer.Ordinal); - var allTemplateStubs = await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false); - var templateNameById = allTemplateStubs.ToDictionary(t => t.Id, t => t.Name); + var templateNameById = await library.GetNameByIdAsync(ct).ConfigureAwait(false); foreach (var tDto in content.Templates) { hydratedByName.TryGetValue(tDto.Name, out var existing); @@ -482,16 +486,26 @@ public sealed class BundleImporter : IBundleImporter } // ---- SharedScripts ---- + // One read of each library (04/P4); DetectBlockersAsync reuses the same lists + // for its known-resolvable name sets instead of reading them again. + var allSharedScripts = await _templateRepo.GetAllSharedScriptsAsync(ct).ConfigureAwait(false); + var sharedScriptByName = allSharedScripts + .GroupBy(x => x.Name, StringComparer.Ordinal) + .ToDictionary(g => g.Key, g => g.First(), StringComparer.Ordinal); foreach (var s in content.SharedScripts) { - var existing = await _templateRepo.GetSharedScriptByNameAsync(s.Name, ct).ConfigureAwait(false); + sharedScriptByName.TryGetValue(s.Name, out var existing); items.Add(_diff.CompareSharedScript(s, existing)); } // ---- ExternalSystems (+ their methods) ---- + var allExternalSystems = await _externalRepo.GetAllExternalSystemsAsync(ct).ConfigureAwait(false); + var externalSystemByName = allExternalSystems + .GroupBy(x => x.Name, StringComparer.Ordinal) + .ToDictionary(g => g.Key, g => g.First(), StringComparer.Ordinal); foreach (var es in content.ExternalSystems) { - var existing = await _externalRepo.GetExternalSystemByNameAsync(es.Name, ct).ConfigureAwait(false); + externalSystemByName.TryGetValue(es.Name, out var existing); IReadOnlyList? methods = null; if (existing is not null) { @@ -622,15 +636,25 @@ public sealed class BundleImporter : IBundleImporter } // ---- Instances (hydrated target + resolved template/site/area names; review item I2) ---- + // GetInstancesByUniqueNamesAsync eagerly Includes all four child nav + // collections (AttributeOverrides / AlarmOverrides / ConnectionBindings / + // NativeAlarmSourceOverrides), so the entity handed to CompareInstance is + // HYDRATED — its children diff correctly instead of every incoming child + // reading as an addition (review item I2). Review 2026-10-21 04/P4: one + // batched read for every bundled instance, and the template / site / area + // names come from the library projection, one site list and one area list + // per site — not four queries per instance. + var existingInstanceByName = content.Instances.Count == 0 + ? new Dictionary(StringComparer.Ordinal) + : (await _templateRepo + .GetInstancesByUniqueNamesAsync(content.Instances.Select(i => i.UniqueName), ct) + .ConfigureAwait(false)) + .ToDictionary(i => i.UniqueName, i => i, StringComparer.Ordinal); + Dictionary? siteIdentifierById = null; + var areaNameByIdBySite = new Dictionary>(); foreach (var instDto in content.Instances) { - // GetInstanceByUniqueNameAsync eagerly Includes all four child nav - // collections (AttributeOverrides / AlarmOverrides / ConnectionBindings / - // NativeAlarmSourceOverrides), so the entity handed to CompareInstance is - // HYDRATED — its children diff correctly instead of every incoming child - // reading as an addition (review item I2). - var existing = await _templateRepo - .GetInstanceByUniqueNameAsync(instDto.UniqueName, ct).ConfigureAwait(false); + existingInstanceByName.TryGetValue(instDto.UniqueName, out var existing); string? existingTemplateName = null; string? existingSiteIdentifier = null; @@ -640,16 +664,21 @@ public sealed class BundleImporter : IBundleImporter // The entity stores template/site/area as numeric FKs that can't be // compared cross-environment, so resolve each to the same stable // string identity the incoming DTO carries. - var tmpl = await _templateRepo.GetTemplateByIdAsync(existing.TemplateId, ct).ConfigureAwait(false); - existingTemplateName = tmpl?.Name; + existingTemplateName = templateNameById.TryGetValue(existing.TemplateId, out var tn) ? tn : null; - var site = await _siteRepo.GetSiteByIdAsync(existing.SiteId, ct).ConfigureAwait(false); - existingSiteIdentifier = site?.SiteIdentifier; + siteIdentifierById ??= (await _siteRepo.GetAllSitesAsync(ct).ConfigureAwait(false)) + .ToDictionary(x => x.Id, x => x.SiteIdentifier); + existingSiteIdentifier = siteIdentifierById.TryGetValue(existing.SiteId, out var si) ? si : null; if (existing.AreaId is int areaId) { - var area = await _templateRepo.GetAreaByIdAsync(areaId, ct).ConfigureAwait(false); - existingAreaName = area?.Name; + if (!areaNameByIdBySite.TryGetValue(existing.SiteId, out var areaNames)) + { + areaNames = (await _templateRepo.GetAreasBySiteIdAsync(existing.SiteId, ct).ConfigureAwait(false)) + .ToDictionary(a => a.Id, a => a.Name); + areaNameByIdBySite[existing.SiteId] = areaNames; + } + existingAreaName = areaNames.TryGetValue(areaId, out var an) ? an : null; } } @@ -662,7 +691,8 @@ public sealed class BundleImporter : IBundleImporter content, ResolveTargetSiteAsync, ResolveTargetConnectionsAsync).ConfigureAwait(false); // ---- Blocker detection ---- - items.AddRange(await DetectBlockersAsync(content, ResolveTargetConnectionsAsync, ct).ConfigureAwait(false)); + items.AddRange(await DetectBlockersAsync( + content, ResolveTargetConnectionsAsync, library, allSharedScripts, allExternalSystems, ct).ConfigureAwait(false)); return new ImportPreview(sessionId, items, requiredSites, requiredConnections); } @@ -776,6 +806,9 @@ public sealed class BundleImporter : IBundleImporter private async Task> DetectBlockersAsync( BundleContentDto content, Func>> resolveTargetConnections, + TemplateLibrary library, + IReadOnlyList allSharedScripts, + IReadOnlyList allExternalSystems, CancellationToken ct) { var blockers = new List(); @@ -790,7 +823,9 @@ public sealed class BundleImporter : IBundleImporter // its new name, but at preview time no resolution has been chosen yet, so // the in-bundle name is the original DTO name — which is what an instance // references. (Explicit rename remap is a D-wave apply-time concern.) - var targetTemplates = await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false); + // Members + lock flags are needed below (CollectLockedOverrideWarnings) but + // never script bodies — the body-free, untracked analysis projection (04/P4). + var targetTemplates = await library.GetAnalysisAsync(ct).ConfigureAwait(false); var targetTemplateNames = new HashSet(StringComparer.Ordinal); foreach (var t in targetTemplates) { @@ -863,8 +898,6 @@ public sealed class BundleImporter : IBundleImporter } // Known-resolvable names = (in-bundle) ∪ (already-in-target). - var allSharedScripts = await _templateRepo.GetAllSharedScriptsAsync(ct).ConfigureAwait(false); - var allExternalSystems = await _externalRepo.GetAllExternalSystemsAsync(ct).ConfigureAwait(false); var sharedScriptNames = new HashSet(StringComparer.Ordinal); foreach (var s in content.SharedScripts) sharedScriptNames.Add(s.Name); @@ -1723,8 +1756,16 @@ public sealed class BundleImporter : IBundleImporter // Skip-resolved DTOs are excluded from the in-bundle name set so // a Skip on a dependency surfaces as a missing-reference error // rather than silently passing. + // Review 2026-10-21 04/P4 + 08/P1: the template library is read ONCE per + // apply attempt — here, before anything is staged — as the untracked, + // body-free analysis projection, and threaded through every pass that only + // needs ids / names / members. The full tracked graph is loaded only for the + // templates this bundle Overwrites (ApplyTemplatesAsync). + var library = new TemplateLibrary(_templateRepo); + await library.GetAnalysisAsync(ct).ConfigureAwait(false); + var (validationErrors, validationWarnings) = - await RunSemanticValidationAsync(content, resolutionMap, ct).ConfigureAwait(false); + await RunSemanticValidationAsync(content, resolutionMap, library, ct).ConfigureAwait(false); // Validate every site / connection / template reference the // site-instance payload depends on BEFORE any row is staged. Running // this in the validation phase (not as an apply-pass guard) preserves @@ -1735,7 +1776,7 @@ public sealed class BundleImporter : IBundleImporter // guards, but in normal operation those never fire. validationErrors = validationErrors.Count > 0 ? validationErrors - : await ValidateSiteInstanceReferencesAsync(content, resolutionMap, nameMap, ct).ConfigureAwait(false); + : await ValidateSiteInstanceReferencesAsync(content, resolutionMap, nameMap, library, ct).ConfigureAwait(false); // Review 2026-10-21 03/R-1 + lane-F follow-up: values the runtime later // trusts blindly — site gRPC endpoints and names that become actor names — @@ -1818,8 +1859,8 @@ public sealed class BundleImporter : IBundleImporter // write itself. await _dbContext.SaveChangesAsync(ct).ConfigureAwait(false); await ResolveAlarmScriptLinksAsync(content.Templates, resolutionMap, user, ct).ConfigureAwait(false); - await ResolveCompositionEdgesAsync(content.Templates, resolutionMap, user, ct).ConfigureAwait(false); - await ResolveInheritanceEdgesAsync(content.Templates, resolutionMap, user, ct).ConfigureAwait(false); + await ResolveCompositionEdgesAsync(content.Templates, resolutionMap, library, user, ct).ConfigureAwait(false); + await ResolveInheritanceEdgesAsync(content.Templates, resolutionMap, library, user, ct).ConfigureAwait(false); // Folder parent edges are name-keyed too — resolve them AFTER the flush // above so every imported folder has a materialised id to point at // (the Add pass created the folders parent-less). Mirrors the template @@ -1842,7 +1883,7 @@ public sealed class BundleImporter : IBundleImporter // catch below rolls the whole import back (ChangeTracker.Clear on the // in-memory provider) BEFORE any of it is persisted; deliberately no // SaveChanges precedes this check so nothing survives that rollback. - EnsureNoTemplateGraphCycles(); + EnsureNoTemplateGraphCycles(await library.GetAnalysisAsync(ct).ConfigureAwait(false)); // ---- Site/instance-scoped apply: instances LAST ---- // The instance pass needs every FK target materialised first: @@ -1857,7 +1898,7 @@ public sealed class BundleImporter : IBundleImporter // rewires every name-keyed FK to the target environment's ids. await ApplyInstancesAsync( content, resolutionMap, siteBySourceIdentifier, connectionMaps, - user, summary, ct).ConfigureAwait(false); + library, user, summary, ct).ConfigureAwait(false); // ---- Pre-commit point: stale-instance computation ---- // Everything the import will write is staged on the change tracker at @@ -1869,7 +1910,7 @@ public sealed class BundleImporter : IBundleImporter // is the next statement, so a read against the change tracker here sees // the full post-apply graph before commit. var staleInstanceIds = await ComputeStaleInstanceIdsAsync( - content, resolutionMap, ct).ConfigureAwait(false); + content, resolutionMap, library, ct).ConfigureAwait(false); await _auditService.LogAsync( user: user, @@ -2012,14 +2053,21 @@ public sealed class BundleImporter : IBundleImporter /// /// Enumerates the deployed instances stale-ed by this import. /// Overwriting a template changes the flattened-config hash of every instance - /// of that template, so a deployed instance's freshly-computed revision hash + /// built from it, so a deployed instance's freshly-computed revision hash /// will no longer match the hash captured in its DeployedConfigSnapshot - /// at deploy time — it surfaces as "stale" on the Deployments page. This walks - /// the templates THIS import OVERWROTE, finds each one's deployed instances + /// at deploy time — it surfaces as "stale" on the Deployments page. This starts + /// from the templates THIS import OVERWROTE and — review 2026-10-21 04/C2 — + /// expands that set over the post-apply template graph to every template whose + /// flattened config includes an overwritten one: descendants through + /// ParentTemplateId and templates composing it through + /// TemplateComposition.ComposedTemplateId, transitively (a template that + /// composes a derived template of an overwritten base is affected too). It then + /// finds each affected template's deployed instances /// (State / - /// with a snapshot), recomputes their current hash through - /// (which reads the just-staged template - /// rows on the shared change tracker), and collects the ids whose hash drifted. + /// with a snapshot), recomputes their current hash through ONE batch of + /// (a single flatten session, so a shared + /// template chain is walked once — the probe reads the just-staged template rows + /// on the shared change tracker), and collects the ids whose hash drifted. /// /// Freshly-IMPORTED instances are excluded structurally: they enter the target /// (no snapshot), so they're "new", not @@ -2032,6 +2080,7 @@ public sealed class BundleImporter : IBundleImporter private async Task> ComputeStaleInstanceIdsAsync( BundleContentDto content, Dictionary<(string EntityType, string Name), ImportResolution> resolutionMap, + TemplateLibrary library, CancellationToken ct) { // No flattening pipeline registered → can't recompute hashes. Staleness is @@ -2043,11 +2092,10 @@ public sealed class BundleImporter : IBundleImporter // Which templates did THIS import overwrite? A bundle template is // overwritten when its resolution Action == Overwrite AND a target template - // of that name already exists (otherwise the Overwrite branch falls through - // to Add — see ApplyTemplatesAsync). Map those names to their target ids. - var targetTemplateIdByName = (await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false)) - .GroupBy(t => t.Name, StringComparer.Ordinal) - .ToDictionary(g => g.Key, g => g.First().Id, StringComparer.Ordinal); + // of that name already existed (otherwise the Overwrite branch falls through + // to Add — see ApplyTemplatesAsync). The pre-write library maps those names + // to their (unchanged) target ids. + var targetTemplateIdByName = await library.GetIdByNameAsync(ct).ConfigureAwait(false); var overwrittenTemplateIds = new HashSet(); foreach (var dto in content.Templates) @@ -2065,57 +2113,118 @@ public sealed class BundleImporter : IBundleImporter return Array.Empty(); } - var staleIds = new List(); - foreach (var templateId in overwrittenTemplateIds) + var affectedTemplateIds = ExpandToDependentTemplates( + overwrittenTemplateIds, MergeTemplateGraph(await library.GetAnalysisAsync(ct).ConfigureAwait(false))); + + // Candidate instances: deployed instances of any affected template. The + // repository read is tracked, so an instance this import just reset to + // NotDeployed reads back with its staged state and is skipped as "new". + var candidates = new List<(int InstanceId, int TemplateId)>(); + foreach (var templateId in affectedTemplateIds) { var instances = await _templateRepo .GetInstancesByTemplateIdAsync(templateId, ct).ConfigureAwait(false); foreach (var instance in instances) { - // NotDeployed instances (including everything this import just - // created/overwrote) are "new", not "stale" — skip them. - if (instance.State is not (InstanceState.Enabled or InstanceState.Disabled)) + if (instance.State is InstanceState.Enabled or InstanceState.Disabled) { - continue; - } - - try - { - // A deployed instance has a snapshot; its absence means the - // instance is not actually deployed (defensive — State already - // gated this) so there's no baseline to compare against. - var snapshot = await _dbContext.DeployedConfigSnapshots - .AsNoTracking() - .Where(s => s.InstanceId == instance.Id) - .OrderByDescending(s => s.DeployedAt) - .FirstOrDefaultAsync(ct) - .ConfigureAwait(false); - if (snapshot is null) continue; - - var currentHash = await _staleInstanceProbe - .GetCurrentRevisionHashAsync(instance.Id, ct).ConfigureAwait(false); - // null => hash indeterminate; don't claim staleness either way. - if (currentHash is null) continue; - - if (!string.Equals(currentHash, snapshot.RevisionHash, StringComparison.Ordinal)) - { - staleIds.Add(instance.Id); - } - } - catch (Exception ex) - { - // Best-effort: a single instance's hash failure must not abort - // the (already-successful) import. Skip it and move on. - _logger?.LogWarning(ex, - "Stale-instance hash computation failed for instance {InstanceId} (template {TemplateId}); skipping.", - instance.Id, templateId); + candidates.Add((instance.Id, templateId)); } } } + if (candidates.Count == 0) + { + return Array.Empty(); + } + + // Latest snapshot hash per candidate in one projected read (no config JSON). + // A deployed instance has a snapshot; its absence means the instance is not + // actually deployed (defensive — State already gated this), so there is no + // baseline to compare against. + var candidateIds = candidates.Select(c => c.InstanceId).ToList(); + var snapshotRows = await _dbContext.DeployedConfigSnapshots + .AsNoTracking() + .Where(s => candidateIds.Contains(s.InstanceId)) + .Select(s => new { s.InstanceId, s.DeployedAt, s.RevisionHash }) + .ToListAsync(ct) + .ConfigureAwait(false); + var deployedHashByInstance = snapshotRows + .GroupBy(r => r.InstanceId) + .ToDictionary(g => g.Key, g => g.OrderByDescending(r => r.DeployedAt).First().RevisionHash); + + var probe = _staleInstanceProbe.BeginBatch(); + var staleIds = new List(); + foreach (var (instanceId, templateId) in candidates) + { + if (!deployedHashByInstance.TryGetValue(instanceId, out var deployedHash)) continue; + try + { + var currentHash = await probe.GetCurrentRevisionHashAsync(instanceId, ct).ConfigureAwait(false); + // null => hash indeterminate; don't claim staleness either way. + if (currentHash is null) continue; + + if (!string.Equals(currentHash, deployedHash, StringComparison.Ordinal)) + { + staleIds.Add(instanceId); + } + } + catch (Exception ex) + { + // Best-effort: a single instance's hash failure must not abort + // the (already-successful) import. Skip it and move on. + _logger?.LogWarning(ex, + "Stale-instance hash computation failed for instance {InstanceId} (template {TemplateId}); skipping.", + instanceId, templateId); + } + } + return staleIds; } + /// + /// Review 2026-10-21 04/C2: closes over the + /// "my flattened config includes yours" relation — a template depends on its + /// base (ParentTemplateId) and on every module it composes — so the result + /// is every template whose flattened output can change when a seed changes. + /// + /// The templates whose content changed. + /// The post-apply template graph. + /// The seeds plus every transitive dependent. + internal static HashSet ExpandToDependentTemplates( + IEnumerable seedTemplateIds, IReadOnlyList