From 099728f05a8ccd982272d2b683f1cfa9ba63ddb5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 8 Jul 2026 15:17:47 -0400 Subject: [PATCH] =?UTF-8?q?fix(transport):=20wire=20template=20inheritance?= =?UTF-8?q?=20edges=20on=20bundle=20import=20(C3)=20=E2=80=94=20derived=20?= =?UTF-8?q?templates=20no=20longer=20land=20as=20roots?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/requirements/Component-Transport.md | 7 +- .../Import/BundleImporter.cs | 86 ++++++++ .../Import/InheritanceImportTests.cs | 204 ++++++++++++++++++ 3 files changed, 295 insertions(+), 2 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/InheritanceImportTests.cs diff --git a/docs/requirements/Component-Transport.md b/docs/requirements/Component-Transport.md index 7e4d43e2..77e34a84 100644 --- a/docs/requirements/Component-Transport.md +++ b/docs/requirements/Component-Transport.md @@ -27,7 +27,7 @@ As of M8 (T18), Transport is no longer limited to central-only configuration: it - Manage in-memory `BundleSession` objects: 30-minute TTL, 3-strike passphrase lockout per session. - Compute a per-artifact diff between bundle contents and the target environment, classifying each artifact as Identical, Modified, New, or a Blocker. - Apply user-supplied conflict resolutions (Add, Overwrite, Skip, Rename) in a single EF transaction, running two-tier semantic validation before committing: a minimal name-resolution scan over the merged target (fails fast on unresolved SharedScript / ExternalSystem identifiers), then the full `SemanticValidator` from `ZB.MOM.WW.ScadaBridge.TemplateEngine` over each imported template's per-template `FlattenedConfiguration`. -- Emit `BundleExported`, `BundleImported`, `BundleImportFailed`, `UnencryptedBundleExport`, `BundleImportUnlockFailed`, `BundleImportAlarmScriptUnresolved`, and `BundleImportCompositionUnresolved` audit events via `IAuditService`. +- Emit `BundleExported`, `BundleImported`, `BundleImportFailed`, `UnencryptedBundleExport`, `BundleImportUnlockFailed`, `BundleImportAlarmScriptUnresolved`, `BundleImportCompositionUnresolved`, and `BundleImportBaseTemplateUnresolved` audit events via `IAuditService`. - Thread a `BundleImportId` correlation GUID through every per-entity `AuditLogEntry` written during `ApplyAsync` via a scoped `IAuditCorrelationContext`. - Enforce `RequireDesign` on export and `RequireAdmin` on import both at the Razor page layer and inside the service entrypoints (defense in depth). @@ -284,6 +284,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 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. + **FK rewiring at apply.** Inside the single EF transaction the importer resolves or creates the target sites and connections **first**, then upserts instances (forced to `NotDeployed`) and rewires their foreign keys against the resolved targets: - `InstanceConnectionBinding.DataConnectionId` is resolved via the connection map, with a **Pass-2** that also binds connections which already exist in the target but were not carried in the bundle. @@ -338,6 +340,7 @@ Import flows through the same audited repository methods the UI and CLI use, so | API key added | `ApiKeyCreated` | | Imported alarm references missing on-trigger script | `BundleImportAlarmScriptUnresolved` (warning; alarm FK left null) | | Imported template's composition references missing target template | `BundleImportCompositionUnresolved` (warning; composition row not written) | +| Imported derived template's base template missing (unresolved / skipped) | `BundleImportBaseTemplateUnresolved` (warning; `ParentTemplateId` left null) | **Correlation:** every per-entity row written during an import carries a new optional `BundleImportId` column (the GUID of the parent `BundleImported` summary row). The repository layer (`CentralUiRepository.QueryConfigurationAuditAsync`) already accepts a `BundleImportId` filter parameter. The Configuration Audit Log Viewer UI surface — a "Bundle Import" filter dropdown and a hyperlink on the `BundleImported` summary row that pre-populates that filter — is a deferred UI follow-up (tracked under Transport-012 in the code review backlog). Until then, an operator can drive the same filter via the CLI `audit query --bundle-import-id `. @@ -398,7 +401,7 @@ Exit codes follow the project convention: `0` = success, `1` = command failure ( - **Management Service / CLI** — `ManagementActor` registers three Transport command handlers (`ExportBundleCommand`, `PreviewBundleCommand`, `ImportBundleCommand`) and the CLI ships `bundle export` / `bundle preview` / `bundle import` subcommands. Bundle bytes ride the existing `/management` JSON envelope as base64. - **Deployment Manager** — Never directly invoked by Transport. Transport-driven template changes propagate to deployed instances through the existing revision-hash drift detection in `DeploymentService.CompareAsync`; the Deployments page surfaces affected instances as stale automatically. - **Security & Auth** — Provides `RequireDesign` and `RequireAdmin` policies from `ZB.MOM.WW.ScadaBridge.Security`, enforced at both the page and service layers. -- **Audit Log (Configuration)** — Writes `BundleExported` / `BundleImported` / `BundleImportFailed` / `UnencryptedBundleExport` / `BundleImportUnlockFailed` rows via `IAuditService`, plus per-import name-resolution warnings `BundleImportAlarmScriptUnresolved` and `BundleImportCompositionUnresolved`; per-entity rows from audited repositories are correlated by `BundleImportId` via `IAuditCorrelationContext`. +- **Audit Log (Configuration)** — Writes `BundleExported` / `BundleImported` / `BundleImportFailed` / `UnencryptedBundleExport` / `BundleImportUnlockFailed` rows via `IAuditService`, plus per-import name-resolution warnings `BundleImportAlarmScriptUnresolved`, `BundleImportCompositionUnresolved`, and `BundleImportBaseTemplateUnresolved`; per-entity rows from audited repositories are correlated by `BundleImportId` via `IAuditCorrelationContext`. --- diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index cd5b30e2..882198a0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -1042,6 +1042,7 @@ public sealed class BundleImporter : IBundleImporter 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); // ---- Site/instance-scoped apply: instances LAST ---- // The instance pass needs every FK target materialised first: @@ -2092,6 +2093,91 @@ public sealed class BundleImporter : IBundleImporter } } + /// + /// Second-pass rewire for template inheritance edges (ParentTemplateId), + /// mirroring . The bundle carries the + /// base by name (TemplateDto.BaseTemplateName); the create/overwrite pass + /// never sets the FK, so without this pass a derived template lands as a root and + /// its inheritance is silently lost (arch-review C3). Runs after templates are + /// flushed so both the derived template and its base are resolvable by name. + /// Never throws — an unresolvable base leaves the edge null and records an audit + /// row, keeping the import all-or-nothing on real faults only. + /// + private async Task ResolveInheritanceEdgesAsync( + IReadOnlyList dtos, + Dictionary<(string, string), ImportResolution> resolutionMap, + string user, + CancellationToken ct) + { + if (dtos.Count == 0) return; + + foreach (var dto in dtos) + { + var resolution = ResolveOrDefault(resolutionMap, "Template", dto.Name); + if (resolution.Action == ResolutionAction.Skip) continue; + + var importedName = resolution.Action == ResolutionAction.Rename + ? (resolution.RenameTo ?? dto.Name) + : dto.Name; + + // The just-imported/updated template is tracked in the Local set. + var template = _dbContext.Templates.Local.FirstOrDefault(t => + string.Equals(t.Name, importedName, StringComparison.Ordinal)); + if (template is null) continue; + + // A root-template bundle — or an Overwrite that dropped the base — + // must clear any stale parent edge on the target row. + if (string.IsNullOrEmpty(dto.BaseTemplateName)) + { + template.ParentTemplateId = null; + continue; + } + + // Resolve the base template's PERSISTED name through the resolution + // map (it may have been renamed on import), then look it up by name — + // first in the tracked Local set (anything imported this run), then in + // the wider target DB (pre-existing rows not staged in this transaction). + var baseResolution = ResolveOrDefault(resolutionMap, "Template", dto.BaseTemplateName); + var baseName = baseResolution.Action == ResolutionAction.Rename + ? (baseResolution.RenameTo ?? dto.BaseTemplateName) + : dto.BaseTemplateName; + + var baseStub = _dbContext.Templates.Local.FirstOrDefault(t => + string.Equals(t.Name, baseName, StringComparison.Ordinal)); + int? baseId = baseStub?.Id; + if (baseId is null) + { + var allTargets = await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false); + baseId = allTargets + .FirstOrDefault(t => string.Equals(t.Name, baseName, StringComparison.Ordinal))?.Id; + } + + if (baseId is null) + { + // Unresolvable base (e.g. it was Skip-resolved and isn't in the + // target). Leave the edge null and record it; never throw. + template.ParentTemplateId = null; + await _auditService.LogAsync( + user, + "BundleImportBaseTemplateUnresolved", + "Template", + "0", + importedName, + new + { + DerivedTemplateName = importedName, + UnresolvedBaseTemplateName = dto.BaseTemplateName, + ResolvedBaseName = baseName, + Reason = "Base template name not present in bundle or target.", + }, + ct).ConfigureAwait(false); + continue; + } + + template.ParentTemplateId = baseId.Value; + } + } + private async Task ApplySharedScriptsAsync( IReadOnlyList dtos, Dictionary<(string, string), ImportResolution> map, diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/InheritanceImportTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/InheritanceImportTests.cs new file mode 100644 index 00000000..c83fd501 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/InheritanceImportTests.cs @@ -0,0 +1,204 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +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.Interfaces.Transport; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport; +using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase; +using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories; +using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Services; +using ZB.MOM.WW.ScadaBridge.Transport; + +namespace ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests.Import; + +/// +/// C3 — the import apply path must wire template inheritance edges +/// (ParentTemplateId) from the bundle's name-keyed BaseTemplateName. +/// Before the fix a derived template imported as a root (null parent) — silent +/// data loss. Mirrors CompositionImportTests: full export → load → +/// preview → apply, so the wire-level DTO carries the reference to resolve. +/// +public sealed class InheritanceImportTests : IDisposable +{ + private readonly ServiceProvider _provider; + + public InheritanceImportTests() + { + var services = new ServiceCollection(); + services.AddSingleton( + new ConfigurationBuilder().AddInMemoryCollection().Build()); + + var dbName = $"InheritanceImportTests_{Guid.NewGuid()}"; + services.AddDbContext(opts => opts + .UseInMemoryDatabase(dbName) + .ConfigureWarnings(w => w.Ignore(InMemoryEventId.TransactionIgnoredWarning))); + + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddTransport(); + + _provider = services.BuildServiceProvider(); + } + + public void Dispose() => _provider.Dispose(); + + private async Task ExportAllTemplatesAsync() + { + await using var scope = _provider.CreateAsyncScope(); + var exporter = scope.ServiceProvider.GetRequiredService(); + var ctx = scope.ServiceProvider.GetRequiredService(); + var templateIds = await ctx.Templates.Select(t => t.Id).ToListAsync(); + + var selection = new ExportSelection( + TemplateIds: templateIds, + SharedScriptIds: Array.Empty(), + ExternalSystemIds: Array.Empty(), + DatabaseConnectionIds: Array.Empty(), + NotificationListIds: Array.Empty(), + SmtpConfigurationIds: Array.Empty(), + ApiMethodIds: Array.Empty(), + IncludeDependencies: false); + + var stream = await exporter.ExportAsync(selection, + user: "alice", sourceEnvironment: "dev", + passphrase: null, cancellationToken: CancellationToken.None); + using var ms = new MemoryStream(); + await stream.CopyToAsync(ms); + return ms.ToArray(); + } + + private async Task WipeTemplatesAsync() + { + await using var scope = _provider.CreateAsyncScope(); + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.Templates.RemoveRange(ctx.Templates); + ctx.AuditLogEntries.RemoveRange(ctx.AuditLogEntries); + await ctx.SaveChangesAsync(); + } + + /// Seed base "Pump" + derived "WaterPump" (ParentTemplateId -> Pump). + private async Task SeedBaseAndDerivedAsync() + { + await using var scope = _provider.CreateAsyncScope(); + var ctx = scope.ServiceProvider.GetRequiredService(); + var pump = new Template("Pump") { Description = "base" }; + ctx.Templates.Add(pump); + await ctx.SaveChangesAsync(); + ctx.Templates.Add(new Template("WaterPump") { Description = "derived", ParentTemplateId = pump.Id }); + await ctx.SaveChangesAsync(); + } + + private async Task LoadAsync(byte[] bundleBytes) + { + await using var scope = _provider.CreateAsyncScope(); + var importer = scope.ServiceProvider.GetRequiredService(); + using var ms = new MemoryStream(bundleBytes, writable: false); + var session = await importer.LoadAsync(ms, passphrase: null); + return session.SessionId; + } + + [Fact] + public async Task Import_DerivedTemplate_WiresParentTemplateId() + { + await SeedBaseAndDerivedAsync(); + var bundle = await ExportAllTemplatesAsync(); + await WipeTemplatesAsync(); + + var sessionId = await LoadAsync(bundle); + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var preview = await importer.PreviewAsync(sessionId); + var resolutions = preview.Items + .Select(i => new ImportResolution(i.EntityType, i.Name, ResolutionAction.Add, null)) + .ToList(); + await importer.ApplyAsync(sessionId, resolutions, user: "bob"); + } + + await using (var assert = _provider.CreateAsyncScope()) + { + var ctx = assert.ServiceProvider.GetRequiredService(); + var basePump = await ctx.Templates.SingleAsync(t => t.Name == "Pump"); + var derived = await ctx.Templates.SingleAsync(t => t.Name == "WaterPump"); + Assert.Equal(basePump.Id, derived.ParentTemplateId); // FAILS today: null + } + } + + [Fact] + public async Task Import_Overwrite_UpdatesParentEdge_AndRenamedBaseIsHonoured() + { + // Bundle base renamed to "Pump2" via ResolutionAction.Rename; derived's + // BaseTemplateName ("Pump") must resolve through the rename map to Pump2. + await SeedBaseAndDerivedAsync(); + var bundle = await ExportAllTemplatesAsync(); + await WipeTemplatesAsync(); + + var sessionId = await LoadAsync(bundle); + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var resolutions = new List + { + new("Template", "Pump", ResolutionAction.Rename, "Pump2"), + new("Template", "WaterPump", ResolutionAction.Add, null), + }; + await importer.ApplyAsync(sessionId, resolutions, user: "bob"); + } + + await using (var assert = _provider.CreateAsyncScope()) + { + var ctx = assert.ServiceProvider.GetRequiredService(); + var pump2 = await ctx.Templates.SingleAsync(t => t.Name == "Pump2"); + var derived = await ctx.Templates.SingleAsync(t => t.Name == "WaterPump"); + Assert.Equal(pump2.Id, derived.ParentTemplateId); + } + } + + [Fact] + public async Task Import_DerivedWithSkippedBase_LeavesNullAndAuditsUnresolved() + { + // Base "Pump" is Skip-resolved and absent from the wiped target, so the + // derived's BaseTemplateName cannot resolve — the edge stays null and a + // BundleImportBaseTemplateUnresolved audit row is emitted (never throws). + await SeedBaseAndDerivedAsync(); + var bundle = await ExportAllTemplatesAsync(); + await WipeTemplatesAsync(); + + var sessionId = await LoadAsync(bundle); + ImportResult result; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var resolutions = new List + { + new("Template", "Pump", ResolutionAction.Skip, null), + new("Template", "WaterPump", ResolutionAction.Add, null), + }; + result = await importer.ApplyAsync(sessionId, resolutions, user: "bob"); + } + + await using (var assert = _provider.CreateAsyncScope()) + { + var ctx = assert.ServiceProvider.GetRequiredService(); + var derived = await ctx.Templates.SingleAsync(t => t.Name == "WaterPump"); + Assert.Null(derived.ParentTemplateId); + Assert.False(await ctx.Templates.AnyAsync(t => t.Name == "Pump")); + + var row = Assert.Single(await ctx.AuditLogEntries + .Where(e => e.Action == "BundleImportBaseTemplateUnresolved") + .ToListAsync()); + Assert.Equal(result.BundleImportId, row.BundleImportId); + Assert.Equal("Template", row.EntityType); + Assert.Equal("WaterPump", row.EntityName); + } + } +}