From 0c9dffed78f15e7fe9b8f487c493f33bdd4114a0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 1 Aug 2026 12:32:45 -0400 Subject: [PATCH] =?UTF-8?q?fix(transport):=20flush=20created-site=20ids=20?= =?UTF-8?q?before=20the=20connection=20pass=20=E2=80=94=20create-missing?= =?UTF-8?q?=20import=20failed=20FK=20547=20on=20real=20SQL=20Server?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught live 2026-08-01 importing a bundle into the empty env2 cluster: a create-missing Site has Id == 0 until SaveChanges on a relational provider, and ApplyDataConnectionsAsync stamps DataConnection.SiteId as a raw scalar (no navigation, no EF fix-up), so the insert violated FK_DataConnections_Sites_SiteId. Every importer integration test runs on the EF in-memory provider, which assigns ids eagerly on AddAsync — the exact masking the in-code comment predicted. Fix: one SaveChangesAsync between the site pass and the connection pass, riding the same outer transaction (all-or-nothing preserved; the failed live import rolled back cleanly). Regression test runs the create-missing path on SQLite (CreateMissingSiteRelationalTests) — red without the fix, green with it. --- .../Import/BundleImporter.cs | 8 + .../CreateMissingSiteRelationalTests.cs | 160 ++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/CreateMissingSiteRelationalTests.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 5d514d52..db25a8fd 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -1515,6 +1515,14 @@ public sealed class BundleImporter : IBundleImporter // ids by name.) var siteBySourceIdentifier = await ApplySitesAsync( content, nameMap, resolutionMap, user, summary, ct).ConfigureAwait(false); + // Flush BETWEEN the site pass and the connection pass: a create-missing + // site has Id == 0 until SaveChanges on a relational provider, and the + // connection pass stamps DataConnection.SiteId as a raw scalar (no EF + // navigation, so no fix-up) — without this flush the insert fails the + // FK on real SQL Server (FK_DataConnections_Sites_SiteId, error 547). + // The in-memory provider assigns ids eagerly on AddAsync, which is why + // in-process tests never hit this. Rides the same outer transaction. + await _dbContext.SaveChangesAsync(ct).ConfigureAwait(false); var connectionMaps = await ApplyDataConnectionsAsync( content, nameMap, siteBySourceIdentifier, resolutionMap, user, summary, ct).ConfigureAwait(false); // Flush so site + connection surrogate ids are assigned (relational diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/CreateMissingSiteRelationalTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/CreateMissingSiteRelationalTests.cs new file mode 100644 index 00000000..9fff1d8a --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/CreateMissingSiteRelationalTests.cs @@ -0,0 +1,160 @@ +using Microsoft.AspNetCore.DataProtection; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; +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.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; + +/// +/// Regression guard for the create-missing site/connection apply path on a REAL +/// relational provider (SQLite). Every other importer integration test runs on the +/// EF in-memory provider, which assigns surrogate ids eagerly on AddAsync — +/// masking that a create-missing Site still has Id == 0 until the +/// context is flushed. ApplyDataConnectionsAsync stamps +/// DataConnection.SiteId as a raw scalar (no EF navigation, so no fix-up), +/// so without a flush between the site pass and the connection pass the insert +/// fails the FK on a relational provider (live failure 2026-08-01: SQL Server +/// error 547, FK_DataConnections_Sites_SiteId, on a cross-environment +/// bundle import into an empty target). +/// +public sealed class CreateMissingSiteRelationalTests : IDisposable +{ + private readonly SqliteConnection _connection; + private readonly ServiceProvider _provider; + + public CreateMissingSiteRelationalTests() + { + // One open in-memory SQLite connection for the fixture's lifetime — the + // database lives exactly as long as the connection. + _connection = new SqliteConnection("DataSource=:memory:"); + _connection.Open(); + + var services = new ServiceCollection(); + services.AddSingleton( + new ConfigurationBuilder().AddInMemoryCollection().Build()); + + services.AddDbContext(opts => opts.UseSqlite(_connection)); + + // Secret-bearing columns require the encrypting two-arg ctor — same wiring + // as RoundTripTests / BundleImporterRollbackFailureTests. + services.AddSingleton(new EphemeralDataProtectionProvider()); + services.AddScoped(sp => new ScadaBridgeDbContext( + sp.GetRequiredService>(), + sp.GetRequiredService())); + + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddTransport(); + + _provider = services.BuildServiceProvider(); + + using var scope = _provider.CreateScope(); + scope.ServiceProvider.GetRequiredService().Database.EnsureCreated(); + } + + public void Dispose() + { + _provider.Dispose(); + _connection.Dispose(); + } + + [Fact] + public async Task Import_create_missing_site_and_connection_wires_the_materialised_site_id() + { + // ---- Seed a source site + connection, export the site closure. ---- + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + var site = new Site("Plant 1", "plant-1") + { + Description = "source plant", + NodeAAddress = "akka://site@10.0.0.1:2552", + NodeBAddress = "akka://site@10.0.0.2:2552", + GrpcNodeAAddress = "10.0.0.1:8083", + GrpcNodeBAddress = "10.0.0.2:8083", + }; + ctx.Sites.Add(site); + await ctx.SaveChangesAsync(); + ctx.DataConnections.Add(new DataConnection("OpcUaPrimary", "OpcUa", site.Id) + { + PrimaryConfiguration = "{\"endpoint\":\"opc.tcp://primary\"}", + FailoverRetryCount = 5, + }); + await ctx.SaveChangesAsync(); + } + + Guid sessionId; + 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 bundleStream = await exporter.ExportAsync( + 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), + user: "alice", sourceEnvironment: "dev", + passphrase: null, cancellationToken: CancellationToken.None); + + using var ms = new MemoryStream(); + await bundleStream.CopyToAsync(ms); + ms.Position = 0; + var importer = scope.ServiceProvider.GetRequiredService(); + var session = await importer.LoadAsync(ms, passphrase: null); + sessionId = session.SessionId; + } + + // ---- Wipe the target so the apply exercises CreateNew on both. ---- + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.DataConnections.RemoveRange(ctx.DataConnections); + ctx.Sites.RemoveRange(ctx.Sites); + await ctx.SaveChangesAsync(); + } + + // ---- Apply with create-missing mappings (the CLI's --create-missing-*). ---- + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var nameMap = new BundleNameMap( + Sites: [new SiteMapping("plant-1", MappingAction.CreateNew, null)], + Connections: [new ConnectionMapping("plant-1", "OpcUaPrimary", MappingAction.CreateNew, null)]); + await importer.ApplyAsync( + sessionId, Array.Empty(), user: "bob", + ct: CancellationToken.None, nameMap: nameMap); + } + + // ---- The created connection must reference the created site's REAL id. ---- + await using (var verify = _provider.CreateAsyncScope()) + { + var ctx = verify.ServiceProvider.GetRequiredService(); + var site = Assert.Single(await ctx.Sites.ToListAsync()); + var conn = Assert.Single(await ctx.DataConnections.ToListAsync()); + Assert.True(site.Id > 0, "created site id was never materialised"); + Assert.Equal(site.Id, conn.SiteId); + } + } +}