fix(transport): warn on import of instance overrides targeting locked template members — inert rows no longer silent (plan R2-05 T7)

This commit is contained in:
Joseph Doherty
2026-07-13 10:04:43 -04:00
parent b02c15bbca
commit db623c1435
2 changed files with 273 additions and 2 deletions
@@ -684,8 +684,9 @@ public sealed class BundleImporter : IBundleImporter
// its new name, but at preview time no resolution has been chosen yet, so // its new name, but at preview time no resolution has been chosen yet, so
// the in-bundle name is the original DTO name — which is what an instance // the in-bundle name is the original DTO name — which is what an instance
// references. (Explicit rename remap is a D-wave apply-time concern.) // references. (Explicit rename remap is a D-wave apply-time concern.)
var targetTemplates = await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false);
var targetTemplateNames = new HashSet<string>(StringComparer.Ordinal); var targetTemplateNames = new HashSet<string>(StringComparer.Ordinal);
foreach (var t in await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false)) foreach (var t in targetTemplates)
{ {
targetTemplateNames.Add(t.Name); targetTemplateNames.Add(t.Name);
} }
@@ -883,6 +884,22 @@ public sealed class BundleImporter : IBundleImporter
} }
} }
// N4: advisory warning rows for instance overrides that target a LOCKED
// template member — the flattener drops them, so surface the inert row in the
// wizard (Kind: Warning, non-blocking). resolutionMap is null at preview time,
// so every instance is scanned.
foreach (var (instance, message) in CollectLockedOverrideWarnings(content, resolutionMap: null, targetTemplates))
{
blockers.Add(new ImportPreviewItem(
EntityType: "Instance",
Name: instance,
ExistingVersion: null,
IncomingVersion: null,
Kind: ConflictKind.Warning,
FieldDiffJson: null,
BlockerReason: message));
}
return blockers; return blockers;
} }
@@ -1075,6 +1092,85 @@ public sealed class BundleImporter : IBundleImporter
=> resolutionMap != null => resolutionMap != null
&& ResolveOrDefault(resolutionMap, entityType, name).Action == ResolutionAction.Skip; && ResolveOrDefault(resolutionMap, entityType, name).Action == ResolutionAction.Skip;
/// <summary>
/// Best-effort scan for instance overrides that target a LOCKED template member.
/// The flattener drops such overrides (an override on a locked attribute / alarm /
/// native-alarm-source never takes effect), so the bundle carries an inert row —
/// this surfaces an advisory warning so the operator learns the config won't apply,
/// WITHOUT blocking the import or changing what gets written (bundle fidelity keeps
/// the row; the flattener stays the behavioural authority).
/// <para>
/// Matches locked members by DIRECT name only — own + inherited placeholder rows
/// both carry the <c>IsLocked</c> flag, so a lock on either matches. Overrides
/// addressing composed path-qualified members (<c>y1.z.Val</c>) or derived-shadow
/// locks (<c>LockedInDerived</c> on a base the placeholder row doesn't reflect) may
/// not match a direct row — an unmatched name emits NO warning (never a false
/// positive; the ManagementActor and flattener remain the enforcement points). The
/// instance's template is resolved from the bundle DTO (the version about to be
/// written) first, else the pre-existing target template.
/// </para>
/// </summary>
private static IReadOnlyList<(string Instance, string Message)> CollectLockedOverrideWarnings(
BundleContentDto content,
Dictionary<(string, string), ImportResolution>? resolutionMap,
IReadOnlyList<Template> targetTemplates)
{
var warnings = new List<(string, string)>();
var bundleTemplates = new Dictionary<string, TemplateDto>(StringComparer.Ordinal);
foreach (var t in content.Templates) bundleTemplates[t.Name] = t;
var targetByName = new Dictionary<string, Template>(StringComparer.Ordinal);
foreach (var t in targetTemplates) targetByName[t.Name] = t;
foreach (var inst in content.Instances)
{
if (IsSkipResolution(resolutionMap, "Instance", inst.UniqueName)) continue;
if (string.IsNullOrEmpty(inst.TemplateName)) continue;
HashSet<string> lockedAttrs;
HashSet<string> lockedAlarms;
HashSet<string> lockedSources;
if (bundleTemplates.TryGetValue(inst.TemplateName, out var dto))
{
lockedAttrs = new(dto.Attributes.Where(a => a.IsLocked).Select(a => a.Name), StringComparer.Ordinal);
lockedAlarms = new(dto.Alarms.Where(a => a.IsLocked).Select(a => a.Name), StringComparer.Ordinal);
lockedSources = new(dto.NativeAlarmSources.Where(s => s.IsLocked).Select(s => s.Name), StringComparer.Ordinal);
}
else if (targetByName.TryGetValue(inst.TemplateName, out var tgt))
{
lockedAttrs = new(tgt.Attributes.Where(a => a.IsLocked).Select(a => a.Name), StringComparer.Ordinal);
lockedAlarms = new(tgt.Alarms.Where(a => a.IsLocked).Select(a => a.Name), StringComparer.Ordinal);
lockedSources = new(tgt.NativeAlarmSources.Where(s => s.IsLocked).Select(s => s.Name), StringComparer.Ordinal);
}
else
{
// Template unresolvable — a separate blocker/validation covers that.
continue;
}
foreach (var o in inst.AttributeOverrides)
{
if (lockedAttrs.Contains(o.AttributeName))
warnings.Add((inst.UniqueName, LockedOverrideMessage(inst.UniqueName, "attribute", o.AttributeName)));
}
foreach (var o in inst.AlarmOverrides)
{
if (lockedAlarms.Contains(o.AlarmCanonicalName))
warnings.Add((inst.UniqueName, LockedOverrideMessage(inst.UniqueName, "alarm", o.AlarmCanonicalName)));
}
foreach (var o in inst.NativeAlarmSourceOverrides)
{
if (lockedSources.Contains(o.SourceCanonicalName))
warnings.Add((inst.UniqueName, LockedOverrideMessage(inst.UniqueName, "native-alarm-source", o.SourceCanonicalName)));
}
}
return warnings;
}
private static string LockedOverrideMessage(string unique, string kind, string name) =>
$"Instance '{unique}' {kind} override '{name}' targets a LOCKED template member — " +
"the flattener ignores it, so this part of the bundle's instance config will never take effect.";
/// <summary> /// <summary>
/// Names that look like PascalCase references but are never user-defined /// Names that look like PascalCase references but are never user-defined
/// SharedScripts or ExternalSystems. Filters the false-positive noise the /// SharedScripts or ExternalSystems. Filters the false-positive noise the
@@ -1242,7 +1338,7 @@ public sealed class BundleImporter : IBundleImporter
if (validationWarnings.Count > 0) if (validationWarnings.Count > 0)
{ {
_logger?.LogWarning( _logger?.LogWarning(
"Bundle import {BundleImportId}: {Count} advisory template-script reference warning(s): {Warnings}", "Bundle import {BundleImportId}: {Count} advisory import warning(s): {Warnings}",
bundleImportId, validationWarnings.Count, string.Join("; ", validationWarnings)); bundleImportId, validationWarnings.Count, string.Join("; ", validationWarnings));
} }
@@ -4375,6 +4471,15 @@ public sealed class BundleImporter : IBundleImporter
// (Pass 0 emits only errors), so nothing surfacable is dropped. // (Pass 0 emits only errors), so nothing surfacable is dropped.
if (errors.Count > 0) return (errors, warnings); if (errors.Count > 0) return (errors, warnings);
// N4: advisory warnings for instance overrides that target a LOCKED template
// member. Best-effort, non-blocking (rides ImportResult.Warnings); the row is
// still written (bundle fidelity) — the flattener is the behavioural authority.
var lockScanTemplates = await _templateRepo.GetAllTemplatesAsync(ct).ConfigureAwait(false);
foreach (var (_, message) in CollectLockedOverrideWarnings(content, resolutionMap, lockScanTemplates))
{
warnings.Add(message);
}
// ---- Pass 1: minimal name-resolution scan ---- // ---- Pass 1: minimal name-resolution scan ----
// Build the known-resolvable set. For in-bundle entries, EXCLUDE the // Build the known-resolvable set. For in-bundle entries, EXCLUDE the
@@ -1222,4 +1222,170 @@ public sealed class SiteInstanceImportTests : IDisposable
Assert.Equal(1, result.Added); Assert.Equal(1, result.Added);
} }
// ──────────────────────────────────────────────────────────────────────
// N4 (plan R2-05 T7): warn on overrides of LOCKED template members
// ──────────────────────────────────────────────────────────────────────
[Fact]
public async Task Import_OverrideOnLockedAttribute_EmitsWarning_ImportStillSucceeds()
{
// Template attribute "SetPoint" IsLocked=true; instance carries an
// InstanceAttributeOverride for "SetPoint" — the flattener drops it, so the
// import surfaces an advisory warning but the row IS still written.
await SeedLockedMemberClosureAsync(lockAttribute: true);
var sessionId = await ExportAllSitesAndLoadAsync();
await WipeSiteClosureAsync();
await using (var scope = _provider.CreateAsyncScope())
{
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
ctx.Templates.Add(new Template("Pump") { Description = "pump tpl" });
await ctx.SaveChangesAsync();
}
var nameMap = new BundleNameMap(
Sites: new[] { new SiteMapping("plant-1", MappingAction.CreateNew, null) },
Connections: Array.Empty<ConnectionMapping>());
var result = await ApplyAsync(
sessionId,
new List<ImportResolution>
{
new("Template", "Pump", ResolutionAction.Skip, null),
new("Site", "plant-1", ResolutionAction.Add, null),
new("Instance", "Pump-01", ResolutionAction.Add, null),
},
nameMap);
Assert.Contains(result.Warnings, w =>
w.Contains("SetPoint", StringComparison.Ordinal)
&& w.Contains("locked", StringComparison.OrdinalIgnoreCase));
// Behaviour unchanged: the override row IS still written (bundle fidelity).
await using (var verify = _provider.CreateAsyncScope())
{
var ctx = verify.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
Assert.True(await ctx.InstanceAttributeOverrides.AnyAsync(o => o.AttributeName == "SetPoint"));
}
}
[Fact]
public async Task Preview_OverrideOnLockedNativeAlarmSource_ShowsWarningRow()
{
// Template native-alarm-source "NativeSrc" IsLocked=true; instance carries a
// NativeAlarmSourceOverride for it. The wizard shows a non-blocking Warning row.
await SeedLockedMemberClosureAsync(lockNativeSource: true);
var sessionId = await ExportAllSitesAndLoadAsync();
ImportPreview preview;
await using (var scope = _provider.CreateAsyncScope())
{
var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
preview = await importer.PreviewAsync(sessionId);
}
Assert.Contains(preview.Items, b =>
b.Kind == ConflictKind.Warning && b.EntityType == "Instance"
&& b.BlockerReason!.Contains("locked", StringComparison.OrdinalIgnoreCase));
}
[Fact]
public async Task Import_OverrideOnUnlockedAttribute_NoLockWarning()
{
// Same shape, but "SetPoint" is NOT locked — no lock warning must appear.
await SeedLockedMemberClosureAsync(lockAttribute: false);
var sessionId = await ExportAllSitesAndLoadAsync();
await WipeSiteClosureAsync();
await using (var scope = _provider.CreateAsyncScope())
{
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
ctx.Templates.Add(new Template("Pump") { Description = "pump tpl" });
await ctx.SaveChangesAsync();
}
var nameMap = new BundleNameMap(
Sites: new[] { new SiteMapping("plant-1", MappingAction.CreateNew, null) },
Connections: Array.Empty<ConnectionMapping>());
var result = await ApplyAsync(
sessionId,
new List<ImportResolution>
{
new("Template", "Pump", ResolutionAction.Skip, null),
new("Site", "plant-1", ResolutionAction.Add, null),
new("Instance", "Pump-01", ResolutionAction.Add, null),
},
nameMap);
Assert.DoesNotContain(result.Warnings, w =>
w.Contains("locked", StringComparison.OrdinalIgnoreCase));
}
/// <summary>
/// Seeds a template ("Pump") + site ("plant-1") + instance ("Pump-01") whose
/// overrides target the given members; <paramref name="lockAttribute"/> /
/// <paramref name="lockNativeSource"/> toggle the template member's IsLocked flag.
/// The attribute override is always present; the native-source override (and its
/// connection) are seeded only when a native source is involved.
/// </summary>
private async Task SeedLockedMemberClosureAsync(bool lockAttribute = false, bool lockNativeSource = false)
{
await using var scope = _provider.CreateAsyncScope();
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
var template = new Template("Pump") { Description = "pump tpl" };
template.Attributes.Add(new TemplateAttribute("SetPoint") { Value = "0", IsLocked = lockAttribute });
if (lockNativeSource)
{
template.NativeAlarmSources.Add(new TemplateNativeAlarmSource("NativeSrc")
{
ConnectionName = "OpcUaPrimary",
SourceReference = "ns=3;s=Pump.Alarm",
IsLocked = true,
});
}
ctx.Templates.Add(template);
var site = new Site("Plant 1", "plant-1")
{
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();
DataConnection? conn = null;
if (lockNativeSource)
{
conn = new DataConnection("OpcUaPrimary", "OpcUa", site.Id)
{
PrimaryConfiguration = "{\"endpoint\":\"opc.tcp://primary\"}",
FailoverRetryCount = 5,
};
ctx.DataConnections.Add(conn);
}
var instance = new Instance("Pump-01")
{
TemplateId = template.Id,
SiteId = site.Id,
State = InstanceState.Enabled,
};
instance.AttributeOverrides.Add(new InstanceAttributeOverride("SetPoint") { OverrideValue = "42" });
if (lockNativeSource)
{
instance.NativeAlarmSourceOverrides.Add(new InstanceNativeAlarmSourceOverride("NativeSrc")
{
ConnectionNameOverride = "OpcUaPrimary",
SourceReferenceOverride = "ns=3;s=Pump.Alarm2",
});
}
ctx.Instances.Add(instance);
await ctx.SaveChangesAsync();
}
} }