dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 15 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(transport): case-insensitive name lookups inside a record on import (ScadaBridge#100)

Browse Source 
Item 2 of #100, following the #93/#95 pattern (ToNameLookup: OrdinalIgnoreCase,
first wins, never throws):

- Overwrite child sync matches template attributes, alarms, scripts and native
  alarm sources, and external-system methods, case-insensitively, like their
  (parent, Name) unique indexes under SQL Server's collation. A member that
  differs only in case is updated in place instead of deleted and re-added
  (which lost the row id and could collide with the unique index).
- ResolveAlarmScriptLinksAsync resolves the alarm and its on-trigger script
  case-insensitively; AlarmsEqual compares the script name the same way.
- ArtifactDiff (preview) matches child collections case-insensitively so it
  agrees with the sync; notification recipients keep the ordinal match (no
  unique index, replaced wholesale).
- The rewire passes found the template a DTO was imported onto with an
  ordinal index. Since #93 an Overwrite can match a differently-cased target and
  keeps its spelling, so the passes silently skipped it and the alarm on-trigger
  links the sync had cleared stayed null. TryGetImported adds a case-insensitive
  (base-first) fallback; composed/base template references keep the exact
  lookup (item 1 of #100 is still undecided).

Test: ImportNameLookupTests.Members_inside_a_record_match_case_insensitively,
over a proxy that gives the by-name template reads SQL Server's collation.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 15:09:18 -05:00
parent f5eed5e8fd
commit 6ff5efd3f4
4 changed files with 228 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ZB.MOM.WW.ScadaBridge.Transport/Import/ArtifactDiff.cs
+17 -10
View File
@@ -253,7 +253,8 @@ public sealed class ArtifactDiff
// number added/changed on an existing recipient is a real diff.
(e, i) => e.EmailAddress == i.EmailAddress && e.PhoneNumber == i.PhoneNumber,
"Recipients",
changes);
changes,
ignoreCase: false);
return BuildItem("NotificationList", incoming.Name, changes);
}
@@ -622,12 +623,19 @@ public sealed class ArtifactDiff
Func<TIncoming, string> incomingKey,
Func<TExisting, TIncoming, bool> equal,
string childCategory,
List<FieldChange> changes)
List<FieldChange> changes,
bool ignoreCase = true)
{
var existingByName = existing.GroupBy(existingKey, StringComparer.Ordinal)
.ToDictionary(g => g.Key, g => g.First(), StringComparer.Ordinal);
var incomingByName = incoming.GroupBy(incomingKey, StringComparer.Ordinal)
.ToDictionary(g => g.Key, g => g.First(), StringComparer.Ordinal);
// Child names are unique per parent case-insensitively (the DB unique indexes
// under SQL Server's collation), and the Overwrite sync matches them that way, so
// the diff does too: a case-only difference is the same row (ScadaBridge#100).
// Notification recipients have no such index and are replaced wholesale, so they
// keep the ordinal match (ignoreCase: false).
var comparer = ignoreCase ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal;
var existingByName = existing.GroupBy(existingKey, comparer)
.ToDictionary(g => g.Key, g => g.First(), comparer);
var incomingByName = incoming.GroupBy(incomingKey, comparer)
.ToDictionary(g => g.Key, g => g.First(), comparer);
var added = incomingByName.Keys.Where(k => !existingByName.ContainsKey(k)).OrderBy(k => k, StringComparer.Ordinal).ToList();
var removed = existingByName.Keys.Where(k => !incomingByName.ContainsKey(k)).OrderBy(k => k, StringComparer.Ordinal).ToList();
@@ -659,10 +667,9 @@ public sealed class ArtifactDiff
IEnumerable<TemplateScriptDto> incoming,
List<FieldChange> changes)
{
var existingByName = existing.GroupBy(s => s.Name, StringComparer.Ordinal)
.ToDictionary(g => g.Key, g => g.First(), StringComparer.Ordinal);
var incomingByName = incoming.GroupBy(s => s.Name, StringComparer.Ordinal)
.ToDictionary(g => g.Key, g => g.First(), StringComparer.Ordinal);
// Case-insensitive, as DiffChildren (ScadaBridge#100).
var existingByName = BundleImporter.ToNameLookup(existing, s => s.Name);
var incomingByName = BundleImporter.ToNameLookup(incoming, s => s.Name);
foreach (var name in incomingByName.Keys.Where(k => !existingByName.ContainsKey(k)).OrderBy(k => k, StringComparer.Ordinal))
{
src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs
+63 -22
View File
@@ -2639,8 +2639,11 @@ public sealed class BundleImporter : IBundleImporter
string user,
CancellationToken ct)
{
var existingByName = ex.Attributes.ToDictionary(a => a.Name, a => a, StringComparer.Ordinal);
var dtoByName = dto.Attributes.ToDictionary(a => a.Name, a => a, StringComparer.Ordinal);
// Matched case-insensitively, like the target's unique index on (TemplateId, Name)
// under SQL Server's collation: a member that differs only in case is the same row
// and is updated in place, not deleted and re-added (ScadaBridge#100).
var existingByName = ToNameLookup(ex.Attributes, a => a.Name);
var dtoByName = ToNameLookup(dto.Attributes, a => a.Name);
// Deletes — attributes present on the target but not in the bundle.
foreach (var existing in existingByName.Values.ToList())
@@ -2756,8 +2759,11 @@ public sealed class BundleImporter : IBundleImporter
string user,
CancellationToken ct)
{
var existingByName = ex.Alarms.ToDictionary(a => a.Name, a => a, StringComparer.Ordinal);
var dtoByName = dto.Alarms.ToDictionary(a => a.Name, a => a, StringComparer.Ordinal);
// Matched case-insensitively, like the target's unique index on (TemplateId, Name)
// under SQL Server's collation: a member that differs only in case is the same row
// and is updated in place, not deleted and re-added (ScadaBridge#100).
var existingByName = ToNameLookup(ex.Alarms, a => a.Name);
var dtoByName = ToNameLookup(dto.Alarms, a => a.Name);
// On-trigger script is referenced by name in the bundle; resolve the
// persisted FK back to a name over this template's scripts so the shared
@@ -2883,8 +2889,11 @@ public sealed class BundleImporter : IBundleImporter
string user,
CancellationToken ct)
{
var existingByName = ex.Scripts.ToDictionary(s => s.Name, s => s, StringComparer.Ordinal);
var dtoByName = dto.Scripts.ToDictionary(s => s.Name, s => s, StringComparer.Ordinal);
// Matched case-insensitively, like the target's unique index on (TemplateId, Name)
// under SQL Server's collation: a member that differs only in case is the same row
// and is updated in place, not deleted and re-added (ScadaBridge#100).
var existingByName = ToNameLookup(ex.Scripts, s => s.Name);
var dtoByName = ToNameLookup(dto.Scripts, s => s.Name);
foreach (var existing in existingByName.Values.ToList())
{
@@ -2989,8 +2998,11 @@ public sealed class BundleImporter : IBundleImporter
string user,
CancellationToken ct)
{
var existingByName = ex.NativeAlarmSources.ToDictionary(s => s.Name, s => s, StringComparer.Ordinal);
var dtoByName = dto.NativeAlarmSources.ToDictionary(s => s.Name, s => s, StringComparer.Ordinal);
// Matched case-insensitively, like the target's unique index on (TemplateId, Name)
// under SQL Server's collation: a member that differs only in case is the same row
// and is updated in place, not deleted and re-added (ScadaBridge#100).
var existingByName = ToNameLookup(ex.NativeAlarmSources, s => s.Name);
var dtoByName = ToNameLookup(dto.NativeAlarmSources, s => s.Name);
// Deletes — sources present on the target but not in the bundle.
foreach (var existing in existingByName.Values.ToList())
@@ -3121,7 +3133,7 @@ public sealed class BundleImporter : IBundleImporter
var importedName = resolution.Action == ResolutionAction.Rename
? (resolution.RenameTo ?? dto.Name)
: dto.Name;
if (localByName.TryGetValue(importedName, out var stub))
if (localByName.TryGetImported(importedName, out var stub))
targets.Add((dto, stub));
}
if (targets.Count == 0) return;
@@ -3139,12 +3151,13 @@ public sealed class BundleImporter : IBundleImporter
{
if (string.IsNullOrEmpty(alarmDto.OnTriggerScriptName)) continue;
// Member names are unique per template case-insensitively (ScadaBridge#100).
var alarm = template.Alarms.FirstOrDefault(a =>
string.Equals(a.Name, alarmDto.Name, StringComparison.Ordinal));
string.Equals(a.Name, alarmDto.Name, StringComparison.OrdinalIgnoreCase));
if (alarm is null) continue;
var script = template.Scripts.FirstOrDefault(s =>
string.Equals(s.Name, alarmDto.OnTriggerScriptName, StringComparison.Ordinal));
string.Equals(s.Name, alarmDto.OnTriggerScriptName, StringComparison.OrdinalIgnoreCase));
if (script is null)
{
// Unresolved — emit warning audit row, leave FK null.
@@ -3216,7 +3229,7 @@ public sealed class BundleImporter : IBundleImporter
? (resolution.RenameTo ?? dto.Name)
: dto.Name;
if (localByName.TryGetValue(importedName, out var parentStub))
if (localByName.TryGetImported(importedName, out var parentStub))
targets.Add((dto, parentStub));
}
if (targets.Count == 0) return;
@@ -3340,7 +3353,7 @@ public sealed class BundleImporter : IBundleImporter
: dto.Name;
// The just-imported/updated template is tracked in the Local set.
if (!localByName.TryGetValue(importedName, out var template)) continue;
if (!localByName.TryGetImported(importedName, out var template)) continue;
// A root-template bundle — or an Overwrite that dropped the base —
// must clear any stale parent edge on the target row.
@@ -3396,20 +3409,46 @@ public sealed class BundleImporter : IBundleImporter
/// <summary>
/// Indexes the change tracker's templates by name (ordinal; the first tracked
/// template wins a duplicate, matching the <c>FirstOrDefault</c> scans this
/// replaces). Gitea #47: every <c>DbSet.Local</c> access runs a full
/// replaces). <see cref="LocalTemplateIndex.TryGetImported"/> adds a case-insensitive
/// fallback for the template a bundled DTO itself landed on. Gitea #47: every <c>DbSet.Local</c> access runs a full
/// <c>DetectChanges</c> over the tracker — every template, member and instance
/// the import has staged — so the rewire passes take one snapshot per pass
/// instead of one scan per bundled template (and per composition edge), which
/// made a large import's apply quadratic in its size.
/// </summary>
private Dictionary<string, Template> SnapshotLocalTemplatesByName()
private LocalTemplateIndex SnapshotLocalTemplatesByName() =>
new(_dbContext.Templates.Local.ToList());
/// <summary>
/// The tracked-template name index of <see cref="SnapshotLocalTemplatesByName"/>.
/// </summary>
private sealed class LocalTemplateIndex
{
var byName = new Dictionary<string, Template>(StringComparer.Ordinal);
foreach (var template in _dbContext.Templates.Local)
private readonly Dictionary<string, Template> _exact = new(StringComparer.Ordinal);
private readonly Dictionary<string, Template> _ignoreCase;
public LocalTemplateIndex(IReadOnlyList<Template> tracked)
{
byName.TryAdd(template.Name, template);
foreach (var template in tracked)
{
_exact.TryAdd(template.Name, template);
}
_ignoreCase = ToTemplateLookup(tracked);
}
return byName;
/// <summary>Exact-name lookup (composed / base template references).</summary>
public bool TryGetValue(string name, out Template template) =>
_exact.TryGetValue(name, out template!);
/// <summary>
/// Looks up the template a bundled DTO was imported onto. An Overwrite matches a
/// differently-cased target (ScadaBridge#93) and keeps the target's spelling, so an
/// exact lookup missed it and the rewire passes silently skipped the template —
/// dropping the alarm on-trigger links the child sync had just cleared
/// (ScadaBridge#100). Exact match first, then case-insensitive, base template first.
/// </summary>
public bool TryGetImported(string name, out Template template) =>
_exact.TryGetValue(name, out template!) || _ignoreCase.TryGetValue(name, out template!);
}
/// <summary>
@@ -3728,8 +3767,10 @@ public sealed class BundleImporter : IBundleImporter
var existingMethods = await _externalRepo
.GetMethodsByExternalSystemIdAsync(ex.Id, ct)
.ConfigureAwait(false);
var existingByName = existingMethods.ToDictionary(m => m.Name, m => m, StringComparer.Ordinal);
var dtoByName = dto.Methods.ToDictionary(m => m.Name, m => m, StringComparer.Ordinal);
// Case-insensitive, like the (ExternalSystemDefinitionId, Name) unique index
// (ScadaBridge#100).
var existingByName = ToNameLookup(existingMethods, m => m.Name);
var dtoByName = ToNameLookup(dto.Methods, m => m.Name);
// Deletes — methods present on the target but not in the bundle.
foreach (var existing in existingByName.Values.ToList())
@@ -3889,7 +3930,7 @@ public sealed class BundleImporter : IBundleImporter
if (sys is null) continue;
var existing = await _externalRepo.GetMethodsByExternalSystemIdAsync(sys.Id, ct).ConfigureAwait(false);
var existingNames = existing.Select(m => m.Name).ToHashSet(StringComparer.Ordinal);
var existingNames = existing.Select(m => m.Name).ToHashSet(StringComparer.OrdinalIgnoreCase); // ScadaBridge#100
foreach (var methodDto in dto.Methods)
{
src/ZB.MOM.WW.ScadaBridge.Transport/Import/TemplateChildEquality.cs
+3 -1
View File
@@ -62,7 +62,9 @@ internal static class TemplateChildEquality
&& e.IsLocked == i.IsLocked
&& e.LockedInDerived == i.LockedInDerived
&& e.IsMesReportable == i.IsMesReportable
&& scriptNameById(e.OnTriggerScriptId) == i.OnTriggerScriptName;
// The bundle names the script; the apply resolves it case-insensitively
// (script names are unique per template that way, ScadaBridge#100).
&& string.Equals(scriptNameById(e.OnTriggerScriptId), i.OnTriggerScriptName, StringComparison.OrdinalIgnoreCase);
/// <summary>
/// True when the persisted script equals its bundle DTO across every writable