fix(transport): case-insensitive name lookups, batch preview lookups, correct flattener doc (ScadaBridge#93)

- ApplyTemplatesAsync/PreviewAsync template lookups use a case-insensitive, never-throwing comparer (SQL Server collation); base template wins over derived. Base names are already unique case-insensitively via the filtered unique index on Template.Name, derived templates are not covered by it. - Preview reads database connections, notification lists and API methods with one GetAll query each instead of one by-name query per bundled row. - Component-TemplateEngine.md: the flattener does not call GetAllTemplatesAsync; it loads chains via GetTemplateWithChildrenAsync. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 13:46:45 -05:00
parent bec07378cc
commit c7d868c52e
3 changed files with 223 additions and 10 deletions
@@ -239,7 +239,7 @@ falls back to `GetTemplateChainAsync` — the template's own inheritance chain,
 one load per level — since resyncing a leaf reads only its chain (Gitea #89). The
 read-only resolved-members query (`GetResolvedTemplateMembersCommand`, behind the
 editor's inherited view and CLI `template resolved-members`) reads that chain alone,
-untracked. The flattener keeps reading the whole tracked graph via `GetAllTemplatesAsync`.
+untracked. The flattener does not call `GetAllTemplatesAsync` (verified against `FlatteningPipeline`/`FlatteningService`, Gitea #93): the pipeline loads each template chain the instance needs through `GetTemplateWithChildrenAsync` (tracked, fully included, memoised per `FlattenSession`) and each composition's chain the same way, so it still reads tracked, body-bearing rows — only the whole-library load is not part of it.

 ### Native Alarm Source Resolution

@@ -482,8 +482,7 @@ public sealed class BundleImporter : IBundleImporter
        var hydratedTemplates = await _templateRepo
            .GetTemplatesWithChildrenAsync(bundleTemplateNames, ct)
            .ConfigureAwait(false);
-        var hydratedByName = hydratedTemplates
-            .ToDictionary(t => t.Name, t => t, StringComparer.Ordinal);
+        var hydratedByName = ToTemplateLookup(hydratedTemplates);
        var templateNameById = await library.GetNameByIdAsync(ct).ConfigureAwait(false);
        foreach (var tDto in content.Templates)
        {
@@ -521,16 +520,24 @@ public sealed class BundleImporter : IBundleImporter
        }

        // ---- DatabaseConnections ----
+        // One read of the library instead of one by-name query per bundled row
+        // (ScadaBridge#93); matched case-insensitively like the SQL Server collation.
+        var dbConnectionByName = content.DatabaseConnections.Count == 0
+            ? ToNameLookup(Array.Empty<DatabaseConnectionDefinition>(), x => x.Name)
+            : ToNameLookup(await _externalRepo.GetAllDatabaseConnectionsAsync(ct).ConfigureAwait(false), x => x.Name);
        foreach (var db in content.DatabaseConnections)
        {
-            var existing = await _externalRepo.GetDatabaseConnectionByNameAsync(db.Name, ct).ConfigureAwait(false);
+            dbConnectionByName.TryGetValue(db.Name, out var existing);
            items.Add(_diff.CompareDatabaseConnection(db, existing));
        }

        // ---- NotificationLists ----
+        var notificationListByName = content.NotificationLists.Count == 0
+            ? ToNameLookup(Array.Empty<NotificationList>(), x => x.Name)
+            : ToNameLookup(await _notificationRepo.GetAllNotificationListsAsync(ct).ConfigureAwait(false), x => x.Name);
        foreach (var nl in content.NotificationLists)
        {
-            var existing = await _notificationRepo.GetListByNameAsync(nl.Name, ct).ConfigureAwait(false);
+            notificationListByName.TryGetValue(nl.Name, out var existing);
            items.Add(_diff.CompareNotificationList(nl, existing));
        }

@@ -560,9 +567,12 @@ public sealed class BundleImporter : IBundleImporter
        // from a hash). They are counted, ignored, and reported at apply time.

        // ---- ApiMethods ----
+        var apiMethodByName = content.ApiMethods.Count == 0
+            ? ToNameLookup(Array.Empty<ApiMethod>(), x => x.Name)
+            : ToNameLookup(await _inboundApiRepo.GetAllApiMethodsAsync(ct).ConfigureAwait(false), x => x.Name);
        foreach (var m in content.ApiMethods)
        {
-            var existing = await _inboundApiRepo.GetMethodByNameAsync(m.Name, ct).ConfigureAwait(false);
+            apiMethodByName.TryGetValue(m.Name, out var existing);
            items.Add(_diff.CompareApiMethod(m, existing));
        }

@@ -2313,6 +2323,32 @@ public sealed class BundleImporter : IBundleImporter
        }
    }

+    /// <summary>
+    /// Name lookup that matches the way SQL Server's default case-insensitive
+    /// collation matches (<see cref="StringComparer.OrdinalIgnoreCase"/>) and can never
+    /// throw on a duplicate key: the first row for a name wins. Used wherever a
+    /// by-name DB result is re-keyed in memory (ScadaBridge#93).
+    /// </summary>
+    internal static Dictionary<string, T> ToNameLookup<T>(IEnumerable<T> rows, Func<T, string> name)
+    {
+        var lookup = new Dictionary<string, T>(StringComparer.OrdinalIgnoreCase);
+        foreach (var row in rows)
+        {
+            lookup.TryAdd(name(row), row);
+        }
+        return lookup;
+    }
+
+    /// <summary>
+    /// <see cref="ToNameLookup{T}"/> for templates. Base templates are unique by name
+    /// (case-insensitively) through the filtered unique index on <c>Name</c>, but
+    /// derived templates hold their composition slot name and are NOT in that index,
+    /// so two rows can differ only in case. A base (user-authored) template wins over
+    /// a derived one, which is the row an import actually targets.
+    /// </summary>
+    internal static Dictionary<string, Template> ToTemplateLookup(IEnumerable<Template> rows) =>
+        ToNameLookup(rows.OrderBy(t => t.IsDerived), t => t.Name);
+
    private async Task ApplyTemplatesAsync(
        IReadOnlyList<TemplateDto> dtos,
        Dictionary<(string, string), ImportResolution> map,
@@ -2328,10 +2364,11 @@ public sealed class BundleImporter : IBundleImporter
            .Where(d => ResolveOrDefault(map, "Template", d.Name).Action == ResolutionAction.Overwrite)
            .Select(d => d.Name)
            .ToList();
-        var byName = (overwriteNames.Count == 0
-                ? Array.Empty<Template>()
-                : await _templateRepo.GetTemplatesWithChildrenAsync(overwriteNames, ct).ConfigureAwait(false))
-            .ToDictionary(t => t.Name, t => t, StringComparer.Ordinal);
+        // The by-name query runs under SQL Server's case-insensitive collation, so the
+        // in-memory lookup must match case-insensitively too (ScadaBridge#93).
+        var byName = ToTemplateLookup(overwriteNames.Count == 0
+            ? Array.Empty<Template>()
+            : await _templateRepo.GetTemplatesWithChildrenAsync(overwriteNames, ct).ConfigureAwait(false));

        // Folder ids must be materialised before we wire templates' FolderId
        // FKs. ApplyTemplateFoldersAsync staged the rows via AddFolderAsync
@@ -0,0 +1,176 @@
+using System.Collections.Concurrent;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Diagnostics;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Notifications;
+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.Templates;
+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.Import;
+
+namespace ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests.Import;
+
+/// <summary>
+/// ScadaBridge#93: preview looked database connections, notification lists and API
+/// methods up one name at a time, and re-keyed template by-name results with an
+/// ordinal comparer although SQL Server matches names case-insensitively.
+/// </summary>
+public sealed class ImportNameLookupTests : IDisposable
+{
+    private readonly ServiceProvider _provider;
+    private readonly ConcurrentDictionary<string, int> _calls = new(StringComparer.Ordinal);
+
+    /// <summary>Initializes the test class with counting repositories over an in-memory database.</summary>
+    public ImportNameLookupTests()
+    {
+        var services = new ServiceCollection();
+        services.AddSingleton<Microsoft.AspNetCore.DataProtection.IDataProtectionProvider>(
+            new Microsoft.AspNetCore.DataProtection.EphemeralDataProtectionProvider());
+        services.AddSingleton<IConfiguration>(new ConfigurationBuilder().AddInMemoryCollection().Build());
+        var dbName = $"ImportNameLookupTests_{Guid.NewGuid()}";
+        services.AddDbContext<ScadaBridgeDbContext>(opts => opts
+            .UseInMemoryDatabase(dbName)
+            .ConfigureWarnings(w => w.Ignore(InMemoryEventId.TransactionIgnoredWarning)));
+        services.AddSingleton<ITemplateGraphWatermark, TemplateGraphWatermark>();
+        services.AddScoped<ITemplateEngineRepository, TemplateEngineRepository>();
+        services.AddScoped<IExternalSystemRepository>(sp =>
+            ImportRoundTripBudgetTests.CountingProxy<IExternalSystemRepository>.Wrap(
+                new ExternalSystemRepository(sp.GetRequiredService<ScadaBridgeDbContext>()), _calls));
+        services.AddScoped<INotificationRepository>(sp =>
+            ImportRoundTripBudgetTests.CountingProxy<INotificationRepository>.Wrap(
+                new NotificationRepository(sp.GetRequiredService<ScadaBridgeDbContext>()), _calls));
+        services.AddScoped<IInboundApiRepository>(sp =>
+            ImportRoundTripBudgetTests.CountingProxy<IInboundApiRepository>.Wrap(
+                new InboundApiRepository(sp.GetRequiredService<ScadaBridgeDbContext>()), _calls));
+        services.AddScoped<ISiteRepository, SiteRepository>();
+        services.AddScoped<IAuditCorrelationContext, AuditCorrelationContext>();
+        services.AddScoped<IAuditService, AuditService>();
+        services.AddTransport();
+        _provider = services.BuildServiceProvider();
+    }
+
+    /// <summary>Releases the resources used by this instance.</summary>
+    public void Dispose() => _provider.Dispose();
+
+    private int Calls(string method) => _calls.TryGetValue(method, out var n) ? n : 0;
+
+    /// <summary>
+    /// Preview of a bundle with several database connections, notification lists and
+    /// API methods must read each library once (no per-name query) and must match the
+    /// target's rows case-insensitively, as SQL Server's collation does.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task Preview_batches_name_lookups_and_matches_case_insensitively()
+    {
+        await using (var scope = _provider.CreateAsyncScope())
+        {
+            var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
+            for (var i = 0; i < 4; i++)
+            {
+                ctx.DatabaseConnectionDefinitions.Add(new DatabaseConnectionDefinition($"db-{i}", "Server=s;Database=d;"));
+                ctx.NotificationLists.Add(new NotificationList($"list-{i}"));
+                ctx.ApiMethods.Add(new ApiMethod($"Method{i}", "return 1;"));
+            }
+            await ctx.SaveChangesAsync();
+        }
+
+        Stream bundle;
+        await using (var scope = _provider.CreateAsyncScope())
+        {
+            var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
+            bundle = await scope.ServiceProvider.GetRequiredService<IBundleExporter>().ExportAsync(
+                new ExportSelection(
+                    TemplateIds: Array.Empty<int>(),
+                    SharedScriptIds: Array.Empty<int>(),
+                    ExternalSystemIds: Array.Empty<int>(),
+                    DatabaseConnectionIds: await ctx.DatabaseConnectionDefinitions.Select(x => x.Id).ToListAsync(),
+                    NotificationListIds: await ctx.NotificationLists.Select(x => x.Id).ToListAsync(),
+                    SmtpConfigurationIds: Array.Empty<int>(),
+                    ApiMethodIds: await ctx.ApiMethods.Select(x => x.Id).ToListAsync(),
+                    IncludeDependencies: false),
+                user: "alice", sourceEnvironment: "dev", passphrase: null,
+                cancellationToken: CancellationToken.None);
+        }
+
+        Guid sessionId;
+        await using (var scope = _provider.CreateAsyncScope())
+        {
+            sessionId = (await scope.ServiceProvider.GetRequiredService<IBundleImporter>()
+                .LoadAsync(bundle, passphrase: null)).SessionId;
+        }
+
+        // The target environment spells two of the names with different casing; the
+        // production collation treats them as the SAME row.
+        await using (var scope = _provider.CreateAsyncScope())
+        {
+            var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
+            (await ctx.DatabaseConnectionDefinitions.SingleAsync(x => x.Name == "db-1")).Name = "DB-1";
+            (await ctx.NotificationLists.SingleAsync(x => x.Name == "list-2")).Name = "LIST-2";
+            (await ctx.ApiMethods.SingleAsync(x => x.Name == "Method3")).Name = "METHOD3";
+            await ctx.SaveChangesAsync();
+        }
+        _calls.Clear();
+
+        ImportPreview preview;
+        await using (var scope = _provider.CreateAsyncScope())
+        {
+            preview = await scope.ServiceProvider.GetRequiredService<IBundleImporter>().PreviewAsync(sessionId);
+        }
+
+        Assert.Equal(0, Calls(nameof(IExternalSystemRepository.GetDatabaseConnectionByNameAsync)));
+        Assert.Equal(0, Calls(nameof(INotificationRepository.GetListByNameAsync)));
+        Assert.Equal(0, Calls(nameof(IInboundApiRepository.GetMethodByNameAsync)));
+        Assert.Equal(1, Calls(nameof(IExternalSystemRepository.GetAllDatabaseConnectionsAsync)));
+        Assert.Equal(1, Calls(nameof(INotificationRepository.GetAllNotificationListsAsync)));
+        Assert.Equal(1, Calls(nameof(IInboundApiRepository.GetAllApiMethodsAsync)));
+
+        foreach (var item in preview.Items.Where(i =>
+                     i.EntityType is "DatabaseConnection" or "NotificationList" or "ApiMethod"))
+        {
+            Assert.NotEqual(ConflictKind.New, item.Kind);
+        }
+        Assert.Equal(12, preview.Items.Count(i =>
+            i.EntityType is "DatabaseConnection" or "NotificationList" or "ApiMethod"));
+    }
+
+    /// <summary>
+    /// Two rows whose names differ only in case (possible for derived templates, which
+    /// the filtered unique index on Name does not cover) must not throw, and a base
+    /// template wins over a derived one.
+    /// </summary>
+    [Fact]
+    public void Template_lookup_tolerates_names_differing_only_in_case()
+    {
+        var derived = new Template("Pump") { Id = 1, IsDerived = true };
+        var other = new Template("pump") { Id = 2, IsDerived = true };
+        var baseT = new Template("PUMP") { Id = 3, IsDerived = false };
+
+        var lookup = BundleImporter.ToTemplateLookup(new[] { derived, other, baseT });
+
+        Assert.Single(lookup);
+        Assert.Same(baseT, lookup["pump"]);
+
+        var onlyDerived = BundleImporter.ToTemplateLookup(new[] { derived, other });
+        Assert.Same(derived, onlyDerived["PUMP"]);
+    }
+
+    /// <summary>A lookup built from by-name rows never throws on duplicate case variants.</summary>
+    [Fact]
+    public void Name_lookup_is_case_insensitive_and_first_wins()
+    {
+        var a = new NotificationList("List");
+        var b = new NotificationList("LIST");
+        var lookup = BundleImporter.ToNameLookup(new[] { a, b }, x => x.Name);
+        Assert.Same(a, lookup["list"]);
+    }
+}