diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 6d65ac18..634ace03 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -474,11 +474,18 @@ public sealed class BundleImporter : IBundleImporter } // ---- DataConnections (site-scoped; matched by name within the auto-matched target site) ---- + // C2: connection names are unique only WITHIN a site, so the preview item's + // identity is SITE-QUALIFIED (`{SiteIdentifier}/{Name}`). The diff CONTENT is + // unchanged — only the item Name is qualified (after CompareDataConnection + // returns) so two sites' same-named connections resolve to distinct items and + // the operator's per-item Skip/Overwrite applies to the right site's connection. + // ApplyDataConnectionsAsync looks the resolution up by this same qualified key. foreach (var dcDto in content.DataConnections) { var targetConns = await ResolveTargetConnectionsAsync(dcDto.SiteIdentifier).ConfigureAwait(false); var existing = targetConns.FirstOrDefault(c => string.Equals(c.Name, dcDto.Name, StringComparison.Ordinal)); - items.Add(_diff.CompareDataConnection(dcDto, existing)); + var item = _diff.CompareDataConnection(dcDto, existing); + items.Add(item with { Name = QualifiedConnectionName(dcDto.SiteIdentifier, dcDto.Name) }); } // ---- Instances (hydrated target + resolved template/site/area names; review item I2) ---- @@ -709,9 +716,11 @@ public sealed class BundleImporter : IBundleImporter if (bundleConnections.Contains((site, name))) continue; var targetConns = await resolveTargetConnections(site).ConfigureAwait(false); if (targetConns.Any(c => string.Equals(c.Name, name, StringComparison.Ordinal))) continue; + // C2: site-qualify the blocker Name so two sites' same-named-but-missing + // connections surface as distinct blockers (a bare name would collide). blockers.Add(new ImportPreviewItem( EntityType: "Instance", - Name: name, + Name: QualifiedConnectionName(site, name), ExistingVersion: null, IncomingVersion: null, Kind: ConflictKind.Blocker, @@ -859,6 +868,18 @@ public sealed class BundleImporter : IBundleImporter private static bool IsIdentifierStart(char c) => c == '_' || char.IsLetter(c); private static bool IsIdentifierChar(char c) => c == '_' || char.IsLetterOrDigit(c); + /// + /// C2: the site-qualified identity ({siteIdentifier}/{connectionName}) used + /// for every DataConnection preview item Name + blocker Name, and for the + /// matching resolution lookup in . Connection + /// names are unique only WITHIN a site, so a bare-name key would collapse two sites' + /// same-named connections onto one resolution entry. Callers building a resolution + /// map from preview.Items keyed by (EntityType, Name) get the + /// site-qualified key for free. + /// + private static string QualifiedConnectionName(string siteIdentifier, string connectionName) => + $"{siteIdentifier}/{connectionName}"; + /// public async Task ApplyAsync( Guid sessionId, @@ -2722,8 +2743,12 @@ public sealed class BundleImporter : IBundleImporter } // MapToExisting — honour the connection's own conflict resolution. + // C2: the resolution is keyed by the SITE-QUALIFIED name + // (`{SiteIdentifier}/{Name}`), matching the qualified preview-item Name — + // so a same-named connection under a different site can't pick up the + // wrong Skip/Overwrite. var resolution = ResolveOrDefault( - resolutionMap, "DataConnection", dcDto.Name); + resolutionMap, "DataConnection", QualifiedConnectionName(dcDto.SiteIdentifier, dcDto.Name)); if (resolution.Action == ResolutionAction.Overwrite) { ApplyDataConnectionFields(existing, dcDto); @@ -2743,9 +2768,89 @@ public sealed class BundleImporter : IBundleImporter targetNameByRef[key] = existing.Name; } + // ---- C1 Pass 2 — referenced-but-not-carried connections ---- + // A binding (or native-alarm-source override) can name a connection that + // exists in the TARGET database but was NOT carried in the bundle's + // DataConnections (e.g. exported without it). Preview correctly does NOT + // block it — it auto-matches against the target. But the main loop above + // only populated the maps for connections the bundle CARRIES, so without + // this pass IdBySourceRef would MISS for such a binding and the instance + // pass would write DataConnectionId = 0 (an invalid FK). Mirror the + // site path's Pass 2: for every distinct (sourceSite, connName) referenced + // by an instance binding / native-alarm override that isn't already mapped, + // resolve the target site (honouring a nameMap redirect to a differently- + // named target connection) and look up the EXISTING target connection by + // name, populating both maps with its real id + name. + foreach (var (sourceSite, connName) in EnumerateReferencedConnectionRefs(content)) + { + var key = (sourceSite, connName); + if (result.ContainsKey(key)) continue; // already mapped by the carried-connection loop. + + if (!siteBySourceIdentifier.TryGetValue(sourceSite, out var targetSite)) + { + // ApplySitesAsync resolved every referenced site; guard so a missing + // entry fails the import rather than writing an orphan FK. + throw new InvalidOperationException( + $"Connection '{sourceSite}/{connName}' references a site that could not be " + + "resolved to a target."); + } + + connMappingByRef.TryGetValue(key, out var mapping); + var targetName = mapping?.Action == MappingAction.MapToExisting + && mapping.TargetConnectionName is { Length: > 0 } tn + ? tn + : connName; + + var targetConns = await TargetConnsAsync(targetSite.Id).ConfigureAwait(false); + var existing = targetConns.FirstOrDefault(c => + string.Equals(c.Name, targetName, StringComparison.Ordinal)); + if (existing is null) + { + // Should already be a preview blocker (present in neither bundle nor + // target). Fail with a clear message rather than letting the instance + // pass write DataConnectionId = 0. + throw new InvalidOperationException( + $"Connection '{sourceSite}/{connName}' is referenced by the bundle but is present " + + "in neither the bundle nor the target — cannot resolve a target connection."); + } + + result[key] = existing.Id; + targetNameByRef[key] = existing.Name; + } + return new ResolvedConnectionMaps(result, targetNameByRef); } + /// + /// C1: every distinct (sourceSiteIdentifier, connectionName) pair an instance + /// references — via a or a + /// non-null . + /// Drives the connection-map Pass 2 that resolves references the bundle didn't carry. + /// + private static IEnumerable<(string Site, string Name)> EnumerateReferencedConnectionRefs(BundleContentDto content) + { + var seen = new HashSet<(string, string)>(); + foreach (var inst in content.Instances) + { + foreach (var b in inst.ConnectionBindings) + { + if (!string.IsNullOrEmpty(b.ConnectionName) + && seen.Add((inst.SiteIdentifier, b.ConnectionName))) + { + yield return (inst.SiteIdentifier, b.ConnectionName); + } + } + foreach (var n in inst.NativeAlarmSourceOverrides) + { + if (!string.IsNullOrEmpty(n.ConnectionNameOverride) + && seen.Add((inst.SiteIdentifier, n.ConnectionNameOverride))) + { + yield return (inst.SiteIdentifier, n.ConnectionNameOverride); + } + } + } + } + private static DataConnection BuildDataConnection(DataConnectionDto dto, int siteId) => new(dto.Name, dto.Protocol, siteId) { @@ -3013,10 +3118,24 @@ public sealed class BundleImporter : IBundleImporter } foreach (var b in dto.ConnectionBindings) { - // Resolve ConnectionName → target DataConnectionId. A binding whose - // connection didn't resolve was a preview blocker; default to 0 here - // (the FK constraint / a later deploy gate surfaces it) rather than - // throwing, since the binding's attribute may legitimately be unbound. + // Resolve ConnectionName → target DataConnectionId. After the C1 Pass-2 + // in ApplyDataConnectionsAsync, the map carries an entry for every + // referenced connection — carried in the bundle OR auto-matched in the + // target. A binding whose connection name STILL doesn't resolve is a + // structural error (it should already have been a preview blocker + + // a pre-write validation failure); THROW rather than write + // DataConnectionId = 0, which would be an invalid FK on a relational + // provider and a silently-broken binding on the in-memory one. + // A binding may legitimately carry NO connection name (unbound + // attribute) — only a NON-EMPTY name that fails to resolve is an error. + if (!string.IsNullOrEmpty(b.ConnectionName) + && !connectionMaps.IdBySourceRef.TryGetValue((sourceSiteIdentifier, b.ConnectionName), out _)) + { + throw new InvalidOperationException( + $"Instance '{inst.UniqueName}' binding for attribute '{b.AttributeName}' references " + + $"connection '{sourceSiteIdentifier}/{b.ConnectionName}' which could not be resolved " + + "to a target connection (present in neither bundle nor target)."); + } connectionMaps.IdBySourceRef.TryGetValue((sourceSiteIdentifier, b.ConnectionName), out var connId); inst.ConnectionBindings.Add(new InstanceConnectionBinding(b.AttributeName) { diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs index 5b8ec515..546e1d47 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs @@ -569,7 +569,8 @@ public sealed class BundleImporterPreviewTests : IDisposable // missing template would block — but we wiped the template too, so the // instance also blocks; assert the New site/connection at minimum). Assert.Contains(preview.Items, i => i.EntityType == "Site" && i.Name == "plant-1" && i.Kind == ConflictKind.New); - Assert.Contains(preview.Items, i => i.EntityType == "DataConnection" && i.Name == "OpcUaPrimary" && i.Kind == ConflictKind.New); + // C2: DataConnection preview items are site-qualified ({site}/{name}). + Assert.Contains(preview.Items, i => i.EntityType == "DataConnection" && i.Name == "plant-1/OpcUaPrimary" && i.Kind == ConflictKind.New); } [Fact] @@ -600,13 +601,66 @@ public sealed class BundleImporterPreviewTests : IDisposable // The site + connection match the target exactly → Identical, not New. var siteItem = Assert.Single(preview.Items, i => i.EntityType == "Site" && i.Name == "plant-1"); Assert.Equal(ConflictKind.Identical, siteItem.Kind); - var connItem = Assert.Single(preview.Items, i => i.EntityType == "DataConnection" && i.Name == "OpcUaPrimary"); + // C2: DataConnection preview items are site-qualified ({site}/{name}). + var connItem = Assert.Single(preview.Items, i => i.EntityType == "DataConnection" && i.Name == "plant-1/OpcUaPrimary"); Assert.Equal(ConflictKind.Identical, connItem.Kind); // No blocker — the template + connection both resolve in the target. Assert.DoesNotContain(preview.Items, i => i.Kind == ConflictKind.Blocker && i.EntityType == "Instance"); } + [Fact] + public async Task PreviewAsync_two_sites_same_connection_name_emit_two_distinct_site_qualified_items() + { + // C2: a bundle with plant-1/OpcUaPrimary + plant-2/OpcUaPrimary must surface + // TWO distinct DataConnection preview items, each site-qualified + // ({site}/{name}) — NOT a single collapsed "OpcUaPrimary" item. This is what + // lets the operator (and the apply path) resolve each site's connection + // independently. Hand-pack the bundle so both same-named connections are + // present without a heavy two-site export. + var content = new BundleContentDto( + TemplateFolders: Array.Empty(), + Templates: Array.Empty(), + SharedScripts: Array.Empty(), + ExternalSystems: Array.Empty(), + DatabaseConnections: Array.Empty(), + NotificationLists: Array.Empty(), + SmtpConfigs: Array.Empty(), + ApiMethods: Array.Empty()) + { + Sites = new[] + { + new SiteDto("plant-1", "Plant 1", null, null, null, null, null), + new SiteDto("plant-2", "Plant 2", null, null, null, null, null), + }, + DataConnections = new[] + { + new DataConnectionDto("plant-1", "OpcUaPrimary", "OpcUa", 3, null), + new DataConnectionDto("plant-2", "OpcUaPrimary", "OpcUa", 3, null), + }, + Instances = Array.Empty(), + }; + var bytes = await PackBundleAsync(content); + + ImportPreview preview; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var session = await importer.LoadAsync(new MemoryStream(bytes), passphrase: null); + preview = await importer.PreviewAsync(session.SessionId); + } + + var connItems = preview.Items + .Where(i => i.EntityType == "DataConnection") + .Select(i => i.Name) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + // Two distinct site-qualified items — NOT one bare "OpcUaPrimary". + Assert.Equal(new[] { "plant-1/OpcUaPrimary", "plant-2/OpcUaPrimary" }, connItems); + Assert.DoesNotContain(preview.Items, i => i.EntityType == "DataConnection" && i.Name == "OpcUaPrimary"); + } + [Fact] public async Task PreviewAsync_modified_instance_against_hydrated_target_shows_child_diff_not_all_added() { @@ -714,10 +768,11 @@ public sealed class BundleImporterPreviewTests : IDisposable preview = await importer.PreviewAsync(session.SessionId); } - // The unresolved connection blocks… + // The unresolved connection blocks… (C2: blocker Name is site-qualified + // {site}/{name} so two sites' same-named missing connections don't collide). Assert.Contains(preview.Items, i => i.Kind == ConflictKind.Blocker - && i.Name == "PhantomConn" + && i.Name == "plant-1/PhantomConn" && i.BlockerReason is not null && i.BlockerReason.Contains("PhantomConn", StringComparison.Ordinal)); // …but the template resolves in the target, so the instance is NOT a diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs index 6f097385..2604d5a4 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs @@ -270,7 +270,8 @@ public sealed class SiteInstanceImportTests : IDisposable { new("Template", "Pump", ResolutionAction.Skip, null), // already present new("Site", "plant-1", ResolutionAction.Add, null), - new("DataConnection", "OpcUaPrimary", ResolutionAction.Add, null), + // C2: DataConnection resolutions are keyed by the site-qualified name. + new("DataConnection", "plant-1/OpcUaPrimary", ResolutionAction.Add, null), new("Instance", "Pump-01", ResolutionAction.Add, null), }, nameMap); @@ -378,7 +379,8 @@ public sealed class SiteInstanceImportTests : IDisposable { new("Template", "Pump", ResolutionAction.Skip, null), new("Site", "plant-1", ResolutionAction.Skip, null), // leave target site untouched - new("DataConnection", "OpcUaPrimary", ResolutionAction.Skip, null), // leave target conn untouched + // C2: DataConnection resolutions are keyed by the site-qualified name. + new("DataConnection", "plant-1/OpcUaPrimary", ResolutionAction.Skip, null), // leave target conn untouched new("Instance", "Pump-01", ResolutionAction.Add, null), }, nameMap); @@ -499,7 +501,8 @@ public sealed class SiteInstanceImportTests : IDisposable { new("Template", "Pump", ResolutionAction.Skip, null), new("Site", "plant-1", ResolutionAction.Skip, null), - new("DataConnection", "OpcUaPrimary", ResolutionAction.Skip, null), + // C2: DataConnection resolution is keyed by the SOURCE site-qualified name. + new("DataConnection", "plant-1/OpcUaPrimary", ResolutionAction.Skip, null), new("Instance", "Pump-01", ResolutionAction.Add, null), }, nameMap); @@ -582,7 +585,7 @@ public sealed class SiteInstanceImportTests : IDisposable new List { new("Site", "plant-1", ResolutionAction.Add, null), - new("DataConnection", "OpcUaPrimary", ResolutionAction.Add, null), + new("DataConnection", "plant-1/OpcUaPrimary", ResolutionAction.Add, null), new("Instance", "Pump-01", ResolutionAction.Add, null), }, user: "bob", @@ -641,7 +644,8 @@ public sealed class SiteInstanceImportTests : IDisposable { new("Template", "Pump", ResolutionAction.Skip, null), new("Site", "plant-1", ResolutionAction.Skip, null), - new("DataConnection", "OpcUaPrimary", ResolutionAction.Skip, null), + // C2: DataConnection resolution is keyed by the site-qualified name. + new("DataConnection", "plant-1/OpcUaPrimary", ResolutionAction.Skip, null), new("Instance", "Pump-01", ResolutionAction.Overwrite, null), }, nameMap); @@ -666,4 +670,317 @@ public sealed class SiteInstanceImportTests : IDisposable Assert.Equal(1, result.Overwritten); } + + // ────────────────────────────────────────────────────────────────────── + // C1 — binding references a TARGET connection the bundle did NOT carry + // ────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task ApplyAsync_binding_to_target_connection_omitted_from_bundle_resolves_to_existing_id_not_zero() + { + // C1 regression: a valid bundle can carry an instance whose binding (and + // native-alarm-source override) references a connection that exists in the + // TARGET but was NOT carried in the bundle's DataConnections. Preview does + // NOT block it (it auto-matches the target). Before the C1 Pass-2 fix the + // connection map MISSED for that binding → DataConnectionId defaulted to 0 + // (an invalid FK). After the fix the map resolves the EXISTING target + // connection's id and both the binding + the native-alarm override rewrite + // correctly. Hand-pack the bundle so DataConnections is empty while the + // instance still references the connection by name. + int targetConnId; + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.Templates.Add(new Template("Pump") { Description = "pump tpl" }); + var site = new Site("Plant 1", "plant-1"); + ctx.Sites.Add(site); + await ctx.SaveChangesAsync(); + var conn = new DataConnection("OpcUaPrimary", "OpcUa", site.Id) + { + PrimaryConfiguration = "{\"endpoint\":\"opc.tcp://target-existing\"}", + }; + ctx.DataConnections.Add(conn); + await ctx.SaveChangesAsync(); + targetConnId = conn.Id; + } + + var content = new BundleContentDto( + TemplateFolders: Array.Empty(), + Templates: Array.Empty(), + SharedScripts: Array.Empty(), + ExternalSystems: Array.Empty(), + DatabaseConnections: Array.Empty(), + NotificationLists: Array.Empty(), + SmtpConfigs: Array.Empty(), + ApiMethods: Array.Empty()) + { + // Site carried, but the connection is DELIBERATELY OMITTED from the bundle. + Sites = new[] + { + new SiteDto("plant-1", "Plant 1", null, null, null, null, null), + }, + DataConnections = Array.Empty(), + Instances = new[] + { + new InstanceDto( + UniqueName: "Pump-01", + TemplateName: "Pump", + SiteIdentifier: "plant-1", + AreaName: null, + State: InstanceState.Enabled, + AttributeOverrides: Array.Empty(), + AlarmOverrides: Array.Empty(), + NativeAlarmSourceOverrides: new[] + { + new InstanceNativeAlarmSourceOverrideDto( + SourceCanonicalName: "NativeSrc", + ConnectionNameOverride: "OpcUaPrimary", + SourceReferenceOverride: "ns=3;s=Pump.Alarm", + ConditionFilterOverride: null), + }, + ConnectionBindings: new[] + { + new InstanceConnectionBindingDto( + AttributeName: "Flow", + ConnectionName: "OpcUaPrimary", + DataSourceReferenceOverride: "ns=3;s=Pump.Flow"), + }), + }, + }; + var sessionId = await PackAndLoadAsync(content); + + var nameMap = new BundleNameMap( + Sites: new[] { new SiteMapping("plant-1", MappingAction.MapToExisting, "plant-1") }, + // No explicit connection mapping — the connection auto-matches the + // existing target connection within plant-1 (the C1 Pass-2 path). + Connections: Array.Empty()); + + var result = await ApplyAsync( + sessionId, + new List + { + new("Site", "plant-1", ResolutionAction.Skip, null), + new("Instance", "Pump-01", ResolutionAction.Add, null), + }, + nameMap); + + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + + // No new connection was created — the omitted-from-bundle connection + // auto-matched the EXISTING target row. + var conn = Assert.Single(await ctx.DataConnections.Where(c => c.Name == "OpcUaPrimary").ToListAsync()); + Assert.Equal(targetConnId, conn.Id); + + var inst = await ctx.Instances + .Include(i => i.ConnectionBindings) + .Include(i => i.NativeAlarmSourceOverrides) + .SingleAsync(i => i.UniqueName == "Pump-01"); + + // THE FIX: binding FK points at the EXISTING target connection id — NOT 0. + var binding = Assert.Single(inst.ConnectionBindings); + Assert.NotEqual(0, binding.DataConnectionId); + Assert.Equal(targetConnId, binding.DataConnectionId); + + // Native-alarm-source override connection name carries through. + Assert.Equal("OpcUaPrimary", inst.NativeAlarmSourceOverrides.Single().ConnectionNameOverride); + } + + Assert.Equal(1, result.Added); + } + + [Fact] + public async Task ApplyAsync_binding_to_connection_in_neither_bundle_nor_target_fails_instead_of_writing_zero() + { + // C1 guard: a binding naming a connection present in NEITHER the bundle nor + // the target must FAIL the import (caught in the pre-write validation phase + // as a SemanticValidationException) rather than silently write + // DataConnectionId = 0. Nothing may persist. + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.Templates.Add(new Template("Pump") { Description = "pump tpl" }); + ctx.Sites.Add(new Site("Plant 1", "plant-1")); + await ctx.SaveChangesAsync(); + } + + var content = new BundleContentDto( + TemplateFolders: Array.Empty(), + Templates: Array.Empty(), + SharedScripts: Array.Empty(), + ExternalSystems: Array.Empty(), + DatabaseConnections: Array.Empty(), + NotificationLists: Array.Empty(), + SmtpConfigs: Array.Empty(), + ApiMethods: Array.Empty()) + { + DataConnections = Array.Empty(), + Instances = new[] + { + new InstanceDto( + UniqueName: "Pump-01", + TemplateName: "Pump", + SiteIdentifier: "plant-1", + AreaName: null, + State: InstanceState.Enabled, + AttributeOverrides: Array.Empty(), + AlarmOverrides: Array.Empty(), + NativeAlarmSourceOverrides: Array.Empty(), + ConnectionBindings: new[] + { + new InstanceConnectionBindingDto( + AttributeName: "Flow", + ConnectionName: "GhostConn", + DataSourceReferenceOverride: null), + }), + }, + }; + var sessionId = await PackAndLoadAsync(content); + + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var nameMap = new BundleNameMap( + Sites: new[] { new SiteMapping("plant-1", MappingAction.MapToExisting, "plant-1") }, + Connections: Array.Empty()); + await Assert.ThrowsAsync(() => + importer.ApplyAsync( + sessionId, + new List + { + new("Site", "plant-1", ResolutionAction.Skip, null), + new("Instance", "Pump-01", ResolutionAction.Add, null), + }, + user: "bob", + ct: CancellationToken.None, + nameMap: nameMap)); + } + + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + // No instance with a zero (or any) binding persisted. + Assert.Equal(0, await ctx.Instances.CountAsync()); + Assert.Equal(0, await ctx.InstanceConnectionBindings.CountAsync()); + } + } + + // ────────────────────────────────────────────────────────────────────── + // C2 — two sites with same-named connections resolve independently + // ────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task ApplyAsync_two_sites_same_connection_name_apply_per_site_resolution_independently() + { + // C2 regression: connection names are unique only WITHIN a site, so a bundle + // with plant-1/OpcUaPrimary + plant-2/OpcUaPrimary must NOT collapse onto a + // single resolution. Both target connections exist; the operator Overwrites + // plant-1's and Skips plant-2's. The site-qualified resolution key routes the + // Overwrite to plant-1's connection ONLY and leaves plant-2's untouched. + int plant1ConnId, plant2ConnId; + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + var s1 = new Site("Plant 1", "plant-1"); + var s2 = new Site("Plant 2", "plant-2"); + ctx.Sites.AddRange(s1, s2); + await ctx.SaveChangesAsync(); + var c1 = new DataConnection("OpcUaPrimary", "OpcUa", s1.Id) + { + PrimaryConfiguration = "{\"endpoint\":\"opc.tcp://p1-existing\"}", + FailoverRetryCount = 1, + }; + var c2 = new DataConnection("OpcUaPrimary", "OpcUa", s2.Id) + { + PrimaryConfiguration = "{\"endpoint\":\"opc.tcp://p2-existing\"}", + FailoverRetryCount = 2, + }; + ctx.DataConnections.AddRange(c1, c2); + await ctx.SaveChangesAsync(); + plant1ConnId = c1.Id; + plant2ConnId = c2.Id; + } + + // Hand-pack a bundle carrying both same-named connections, each with a + // DISTINCT incoming protocol-config so an Overwrite is observable. + var content = new BundleContentDto( + TemplateFolders: Array.Empty(), + Templates: Array.Empty(), + SharedScripts: Array.Empty(), + ExternalSystems: Array.Empty(), + DatabaseConnections: Array.Empty(), + NotificationLists: Array.Empty(), + SmtpConfigs: Array.Empty(), + ApiMethods: Array.Empty()) + { + Sites = new[] + { + new SiteDto("plant-1", "Plant 1", null, null, null, null, null), + new SiteDto("plant-2", "Plant 2", null, null, null, null, null), + }, + DataConnections = new[] + { + new DataConnectionDto("plant-1", "OpcUaPrimary", "OpcUa", 9, + new SecretsBlock(new Dictionary + { + ["PrimaryConfiguration"] = "{\"endpoint\":\"opc.tcp://p1-from-bundle\"}", + })), + new DataConnectionDto("plant-2", "OpcUaPrimary", "OpcUa", 9, + new SecretsBlock(new Dictionary + { + ["PrimaryConfiguration"] = "{\"endpoint\":\"opc.tcp://p2-from-bundle\"}", + })), + }, + Instances = Array.Empty(), + }; + var sessionId = await PackAndLoadAsync(content); + + var nameMap = new BundleNameMap( + Sites: new[] + { + new SiteMapping("plant-1", MappingAction.MapToExisting, "plant-1"), + new SiteMapping("plant-2", MappingAction.MapToExisting, "plant-2"), + }, + Connections: new[] + { + new ConnectionMapping("plant-1", "OpcUaPrimary", MappingAction.MapToExisting, "OpcUaPrimary"), + new ConnectionMapping("plant-2", "OpcUaPrimary", MappingAction.MapToExisting, "OpcUaPrimary"), + }); + + var result = await ApplyAsync( + sessionId, + new List + { + new("Site", "plant-1", ResolutionAction.Skip, null), + new("Site", "plant-2", ResolutionAction.Skip, null), + // Per-site resolutions — keyed by the site-qualified name (C2). + new("DataConnection", "plant-1/OpcUaPrimary", ResolutionAction.Overwrite, null), + new("DataConnection", "plant-2/OpcUaPrimary", ResolutionAction.Skip, null), + }, + nameMap); + + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + + // plant-1's connection was Overwritten with the bundle's config. + var c1 = await ctx.DataConnections.SingleAsync(c => c.Id == plant1ConnId); + Assert.Contains("p1-from-bundle", c1.PrimaryConfiguration!); + Assert.Equal(9, c1.FailoverRetryCount); + + // plant-2's same-named connection was LEFT UNTOUCHED by the Skip — the + // bare-name collision bug would have applied plant-1's Overwrite here too. + var c2 = await ctx.DataConnections.SingleAsync(c => c.Id == plant2ConnId); + Assert.Contains("p2-existing", c2.PrimaryConfiguration!); + Assert.Equal(2, c2.FailoverRetryCount); + + // Still exactly two connections — no duplicates created. + Assert.Equal(2, await ctx.DataConnections.CountAsync(c => c.Name == "OpcUaPrimary")); + } + + // One Overwrite (plant-1 conn), three Skips (two sites + plant-2 conn). + Assert.Equal(1, result.Overwritten); + Assert.Equal(3, result.Skipped); + } }