From f40f52950eb27ba2843d4a03b4dbc0aaf7b0cad7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 9 Jul 2026 16:09:56 -0400 Subject: [PATCH] =?UTF-8?q?fix(transport):=20export=20instance=20AreaName?= =?UTF-8?q?=20=E2=80=94=20Area-by-name=20reconciliation=20was=20half-shipp?= =?UTF-8?q?ed=20(exporter=20hardcoded=20null)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj --- .../Export/BundleExporter.cs | 22 +++ .../Serialization/EntityDtos.cs | 9 + .../Serialization/EntitySerializer.cs | 11 +- .../Import/AreaTransportTests.cs | 175 ++++++++++++++++++ 4 files changed, 213 insertions(+), 4 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/AreaTransportTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Export/BundleExporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Export/BundleExporter.cs index 7a4bffc4..0f9c8b8d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Export/BundleExporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Export/BundleExporter.cs @@ -1,4 +1,5 @@ using System.Security.Cryptography; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Transport; @@ -72,6 +73,24 @@ public sealed class BundleExporter : IBundleExporter // folder chain, transitive shared-scripts/external-systems if requested). var resolved = await _resolver.ResolveAsync(selection, cancellationToken).ConfigureAwait(false); + // 1b. Areas travel by NAME (the importer resolves-or-creates them under the + // target site), so we only need an id→name lookup for the AreaIds the + // exported instances actually reference — a single query over Areas keyed + // to those FKs. Without this the serializer would emit AreaName: null and + // the imported instance would lose its area assignment. + var referencedAreaIds = resolved.Instances + .Select(i => i.AreaId) + .Where(id => id is not null) + .Select(id => id!.Value) + .Distinct() + .ToList(); + var areaNameById = referencedAreaIds.Count == 0 + ? new Dictionary() + : await _dbContext.Areas + .Where(a => referencedAreaIds.Contains(a.Id)) + .ToDictionaryAsync(a => a.Id, a => a.Name, cancellationToken) + .ConfigureAwait(false); + // 2. Convert to the wire-shaped DTO (strips EF identity, refs-by-name). // The resolver's site/dataConnection/instance closure is wired // through via object-initializer — without this the aggregate would @@ -97,6 +116,9 @@ public sealed class BundleExporter : IBundleExporter // bundle ships an empty smsConfigs array even when SMS configs were // selected — mirrors the site/instance fix (review item I3). SmsConfigurations = resolved.SmsConfigs, + // Area id→name lookup for the exported instances — feeds the serializer's + // AreaName projection so each instance's area travels by name. + AreaNameById = areaNameById, }; var contentDto = _entitySerializer.ToBundleContent(aggregate); diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntityDtos.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntityDtos.cs index 7ad1e97c..de2eb98f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntityDtos.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntityDtos.cs @@ -41,6 +41,15 @@ public sealed record EntityAggregate( // and never sees null; producers that resolve SMS config opt in via // object-initializer. public IReadOnlyList SmsConfigurations { get; init; } = Array.Empty(); + + // Area id → name lookup for the exported instances. Areas are not carried as a + // first-class bundle collection (they reconcile by NAME under the target site), + // so the aggregate only needs enough to resolve each instance's AreaId FK back to + // a portable name at serialization time. Init-only with an empty default: producers + // that export instances populate it; every other caller keeps compiling and the + // serializer emits AreaName: null when an instance's AreaId isn't in the map. + public IReadOnlyDictionary AreaNameById { get; init; } = + new Dictionary(); } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntitySerializer.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntitySerializer.cs index 1726486c..28e4a75f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntitySerializer.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/EntitySerializer.cs @@ -248,10 +248,13 @@ public sealed class EntitySerializer UniqueName: inst.UniqueName, TemplateName: templateNameById.TryGetValue(inst.TemplateId, out var tn) ? tn : string.Empty, SiteIdentifier: siteIdentifierBySiteId.TryGetValue(inst.SiteId, out var isid) ? isid : string.Empty, - // TODO: Areas don't travel in the bundle yet — EntityAggregate - // carries no Areas collection, so there's no id→name lookup. Emit - // null until area transport is added; the importer leaves AreaId null. - AreaName: null, + // Area travels by NAME — the importer resolves-or-creates the + // equivalent area under the target site (ResolveOrCreateAreaIdAsync). + // Resolve the instance's AreaId FK against the aggregate's id→name + // map; if the FK is null (no area) or unresolved (orphan/absent from + // the map), the name comes through as null and the importer leaves + // AreaId null on the imported instance. + AreaName: inst.AreaId is { } aid && aggregate.AreaNameById.TryGetValue(aid, out var an) ? an : null, State: inst.State, AttributeOverrides: inst.AttributeOverrides.Select(o => new InstanceAttributeOverrideDto( AttributeName: o.AttributeName, diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/AreaTransportTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/AreaTransportTests.cs new file mode 100644 index 00000000..5384ecd0 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/AreaTransportTests.cs @@ -0,0 +1,175 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances; +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.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; +using ZB.MOM.WW.ScadaBridge.Transport.Import; +using ZB.MOM.WW.ScadaBridge.Transport.Serialization; + +namespace ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests.Import; + +/// +/// Integration tests for Area-by-name transport (Task 7): a source instance +/// assigned to an under its site must carry that area's NAME +/// through the exported bundle so the importer's ResolveOrCreateAreaIdAsync +/// machinery can resolve-or-create the equivalent area under the target site. This +/// closes the half-shipped Area transport — the importer's GetOrCreate path was +/// already built + tested, but the exporter hardcoded AreaName: null. +/// +/// Reuses the in-memory host pattern from SiteInstanceImportTests: real +/// repositories, real EF in-memory provider, real Transport pipeline. +/// +/// +public sealed class AreaTransportTests : IDisposable +{ + private readonly ServiceProvider _provider; + + public AreaTransportTests() + { + var services = new ServiceCollection(); + services.AddSingleton( + new ConfigurationBuilder().AddInMemoryCollection().Build()); + + var dbName = $"AreaTransportTests_{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(); + + [Fact] + public async Task Instance_Area_travels_by_name_and_resolves_under_target_site() + { + // ---- Seed a site + instance assigned to an Area "Line-1" ---- + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + + var template = new Template("Pump") { Description = "pump tpl" }; + ctx.Templates.Add(template); + + var site = new Site("Plant 1", "plant-1"); + ctx.Sites.Add(site); + await ctx.SaveChangesAsync(); + + var area = new Area("Line-1") { SiteId = site.Id }; + ctx.Areas.Add(area); + await ctx.SaveChangesAsync(); + + var instance = new Instance("Pump-01") + { + TemplateId = template.Id, + SiteId = site.Id, + AreaId = area.Id, + State = InstanceState.Enabled, + }; + ctx.Instances.Add(instance); + await ctx.SaveChangesAsync(); + } + + // ---- Export the whole site closure ---- + Stream bundleStream; + await using (var scope = _provider.CreateAsyncScope()) + { + var exporter = scope.ServiceProvider.GetRequiredService(); + var ctx = scope.ServiceProvider.GetRequiredService(); + var siteIds = await ctx.Sites.Select(s => s.Id).ToListAsync(); + var selection = new ExportSelection( + TemplateIds: Array.Empty(), + SharedScriptIds: Array.Empty(), + ExternalSystemIds: Array.Empty(), + DatabaseConnectionIds: Array.Empty(), + NotificationListIds: Array.Empty(), + SmtpConfigurationIds: Array.Empty(), + ApiMethodIds: Array.Empty(), + IncludeDependencies: true, + SiteIds: siteIds); + bundleStream = await exporter.ExportAsync(selection, user: "alice", sourceEnvironment: "dev", + passphrase: null, cancellationToken: CancellationToken.None); + } + + using var ms = new MemoryStream(); + await bundleStream.CopyToAsync(ms); + ms.Position = 0; + + // ---- Wipe the source rows so the import creates a fresh target ---- + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.Instances.RemoveRange(ctx.Instances); + ctx.Areas.RemoveRange(ctx.Areas); + ctx.Sites.RemoveRange(ctx.Sites); + ctx.Templates.RemoveRange(ctx.Templates); + await ctx.SaveChangesAsync(); + // The template must exist for the instance to resolve its TemplateId. + ctx.Templates.Add(new Template("Pump") { Description = "pump tpl" }); + await ctx.SaveChangesAsync(); + } + + // ---- Import with a CreateNew site mapping ---- + Guid sessionId; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var session = await importer.LoadAsync(ms, passphrase: null); + sessionId = session.SessionId; + } + + var nameMap = new BundleNameMap( + Sites: new[] { new SiteMapping("plant-1", MappingAction.CreateNew, null) }, + Connections: Array.Empty()); + + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + await importer.ApplyAsync( + sessionId, + new List + { + new("Template", "Pump", ResolutionAction.Skip, null), + new("Site", "plant-1", ResolutionAction.Add, null), + new("Instance", "Pump-01", ResolutionAction.Add, null), + }, + user: "bob", + ct: CancellationToken.None, + nameMap: nameMap); + } + + // ---- Assert: the imported instance's AreaId resolves to an Area named + // "Line-1" under the target site (NOT null). ---- + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + var site = await ctx.Sites.SingleAsync(s => s.SiteIdentifier == "plant-1"); + var inst = await ctx.Instances.SingleAsync(i => i.UniqueName == "Pump-01"); + + Assert.NotNull(inst.AreaId); + var area = await ctx.Areas.SingleAsync(a => a.Id == inst.AreaId); + Assert.Equal("Line-1", area.Name); + Assert.Equal(site.Id, area.SiteId); + } + } +}