fix(transport): wire template inheritance edges on bundle import (C3) — derived templates no longer land as roots

2026-07-08 15:17:47 -04:00
parent 6ff90702f0
commit 099728f05a
3 changed files with 295 additions and 2 deletions
@@ -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 <guid>`.

@@ -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`.

 ---

@@ -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
        }
    }

+    /// <summary>
+    /// Second-pass rewire for template inheritance edges (<c>ParentTemplateId</c>),
+    /// mirroring <see cref="ResolveCompositionEdgesAsync"/>. The bundle carries the
+    /// base by name (<c>TemplateDto.BaseTemplateName</c>); 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.
+    /// </summary>
+    private async Task ResolveInheritanceEdgesAsync(
+        IReadOnlyList<TemplateDto> 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<SharedScriptDto> dtos,
        Dictionary<(string, string), ImportResolution> map,
@@ -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;
+
+/// <summary>
+/// C3 — the import apply path must wire template inheritance edges
+/// (<c>ParentTemplateId</c>) from the bundle's name-keyed <c>BaseTemplateName</c>.
+/// Before the fix a derived template imported as a root (null parent) — silent
+/// data loss. Mirrors <c>CompositionImportTests</c>: full export → load →
+/// preview → apply, so the wire-level DTO carries the reference to resolve.
+/// </summary>
+public sealed class InheritanceImportTests : IDisposable
+{
+    private readonly ServiceProvider _provider;
+
+    public InheritanceImportTests()
+    {
+        var services = new ServiceCollection();
+        services.AddSingleton<IConfiguration>(
+            new ConfigurationBuilder().AddInMemoryCollection().Build());
+
+        var dbName = $"InheritanceImportTests_{Guid.NewGuid()}";
+        services.AddDbContext<ScadaBridgeDbContext>(opts => opts
+            .UseInMemoryDatabase(dbName)
+            .ConfigureWarnings(w => w.Ignore(InMemoryEventId.TransactionIgnoredWarning)));
+
+        services.AddScoped<ITemplateEngineRepository, TemplateEngineRepository>();
+        services.AddScoped<IExternalSystemRepository, ExternalSystemRepository>();
+        services.AddScoped<INotificationRepository, NotificationRepository>();
+        services.AddScoped<IInboundApiRepository, InboundApiRepository>();
+        services.AddScoped<ISiteRepository, SiteRepository>();
+        services.AddScoped<IAuditCorrelationContext, AuditCorrelationContext>();
+        services.AddScoped<IAuditService, AuditService>();
+        services.AddTransport();
+
+        _provider = services.BuildServiceProvider();
+    }
+
+    public void Dispose() => _provider.Dispose();
+
+    private async Task<byte[]> ExportAllTemplatesAsync()
+    {
+        await using var scope = _provider.CreateAsyncScope();
+        var exporter = scope.ServiceProvider.GetRequiredService<IBundleExporter>();
+        var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
+        var templateIds = await ctx.Templates.Select(t => t.Id).ToListAsync();
+
+        var selection = new ExportSelection(
+            TemplateIds: templateIds,
+            SharedScriptIds: Array.Empty<int>(),
+            ExternalSystemIds: Array.Empty<int>(),
+            DatabaseConnectionIds: Array.Empty<int>(),
+            NotificationListIds: Array.Empty<int>(),
+            SmtpConfigurationIds: Array.Empty<int>(),
+            ApiMethodIds: Array.Empty<int>(),
+            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<ScadaBridgeDbContext>();
+        ctx.Templates.RemoveRange(ctx.Templates);
+        ctx.AuditLogEntries.RemoveRange(ctx.AuditLogEntries);
+        await ctx.SaveChangesAsync();
+    }
+
+    /// <summary>Seed base "Pump" + derived "WaterPump" (ParentTemplateId -> Pump).</summary>
+    private async Task SeedBaseAndDerivedAsync()
+    {
+        await using var scope = _provider.CreateAsyncScope();
+        var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
+        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<Guid> LoadAsync(byte[] bundleBytes)
+    {
+        await using var scope = _provider.CreateAsyncScope();
+        var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
+        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<IBundleImporter>();
+            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<ScadaBridgeDbContext>();
+            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<IBundleImporter>();
+            var resolutions = new List<ImportResolution>
+            {
+                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<ScadaBridgeDbContext>();
+            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<IBundleImporter>();
+            var resolutions = new List<ImportResolution>
+            {
+                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<ScadaBridgeDbContext>();
+            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);
+        }
+    }
+}