diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 783d6722..67a2c977 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -1170,10 +1170,7 @@ public class ManagementActor : ReceiveActor /// an ambiguous match gets no hint. /// internal static string CaseMismatchHint(string name, IEnumerable candidates) - { - var matches = candidates.Where(c => string.Equals(c, name, StringComparison.OrdinalIgnoreCase)).Distinct(StringComparer.Ordinal).Take(2).ToList(); - return matches.Count == 1 ? $" (names are case-sensitive; did you mean '{matches[0]}'?)" : string.Empty; - } + => OverrideNameMatching.CaseMismatchHint(name, candidates); private static async Task HandleSetInstanceOverrides(IServiceProvider sp, SetInstanceOverridesCommand cmd, AuthenticatedUser user) { diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/OverrideNameMatching.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/OverrideNameMatching.cs new file mode 100644 index 00000000..d01b6b07 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/OverrideNameMatching.cs @@ -0,0 +1,32 @@ +namespace ZB.MOM.WW.ScadaBridge.TemplateEngine; + +/// +/// ScadaBridge#105: instance override names address a template member by its exact +/// (ordinal) name, because the flattener matches overrides ordinally and silently skips +/// one it cannot resolve. Every override write path (ManagementActor bulk attribute and +/// native-source commands, used by the CLI and the management API; and +/// , used by the Central UI) +/// therefore rejects a name that differs from a member only by case instead of storing a +/// dead override. This helper builds the shared "did you mean" hint for that rejection. +/// +public static class OverrideNameMatching +{ + /// + /// Returns a hint naming the single candidate that equals + /// case-insensitively, or an empty string when there is no such candidate or the + /// case-insensitive match is ambiguous. + /// + /// The override name that did not match any member exactly. + /// The member names the override could address. + /// A hint suffix (leading space, no trailing period), or an empty string. + public static string CaseMismatchHint(string name, IEnumerable candidates) + { + ArgumentNullException.ThrowIfNull(candidates); + var matches = candidates + .Where(c => string.Equals(c, name, StringComparison.OrdinalIgnoreCase)) + .Distinct(StringComparer.Ordinal) + .Take(2) + .ToList(); + return matches.Count == 1 ? $" (names are case-sensitive; did you mean '{matches[0]}'?)" : string.Empty; + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Services/InstanceService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Services/InstanceService.cs index c137db3c..39e5be85 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Services/InstanceService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Services/InstanceService.cs @@ -168,7 +168,9 @@ public class InstanceService var templateAttr = templateAttrs.FirstOrDefault(a => a.Name == attributeName); if (templateAttr == null) return Result.Failure( - $"Attribute '{attributeName}' does not exist in template {instance.TemplateId}. Cannot add new attributes via overrides."); + $"Attribute '{attributeName}' does not exist in template {instance.TemplateId}" + + OverrideNameMatching.CaseMismatchHint(attributeName, templateAttrs.Select(a => a.Name)) + + ". Cannot add new attributes via overrides."); if (templateAttr.IsLocked) return Result.Failure( diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index ec677cb8..c2555e32 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -2831,6 +2831,66 @@ public class ManagementActorTests : TestKit, IDisposable _templateRepo.DidNotReceiveWithAnyArgs().AddInstanceAttributeOverrideAsync(default!, default); } + /// + /// ScadaBridge#105, end to end: drive the real ManagementActor bulk attribute override + /// path (the CLI instance set-overrides / management API path), capture whatever it + /// stores, and run the stored overrides through the real . + /// Invariant: every override the management path accepts is applied by the flattener — + /// none is stored and then silently dropped as unresolved. The exact-case row is the + /// positive control (stored AND applied); the case-variant rows must be refused. + /// + [Theory] + [InlineData("Setpoint", true)] + [InlineData("setpoint", false)] + [InlineData("SETPOINT", false)] + public void SetInstanceOverrides_EveryStoredOverride_IsAppliedByTheFlattener(string requestedName, bool accepted) + { + var template = new Template("PumpTemplate") { Id = 1 }; + template.Attributes.Add(new TemplateAttribute("Setpoint") { Id = 1, TemplateId = 1, DataType = DataType.Double, Value = "0" }); + var instance = new Instance("Pump6") { Id = 6, SiteId = 1, TemplateId = 1 }; + _templateRepo.GetInstanceByIdAsync(6, Arg.Any()).Returns(instance); + _templateRepo.GetAttributesByTemplateIdAsync(1, Arg.Any()) + .Returns(template.Attributes.ToList()); + _templateRepo.GetOverridesByInstanceIdAsync(6, Arg.Any()) + .Returns(new List()); + var stored = new List(); + _templateRepo + .When(r => r.AddInstanceAttributeOverrideAsync(Arg.Any(), Arg.Any())) + .Do(ci => stored.Add(ci.Arg())); + + var actor = CreateActor(); + actor.Tell(Envelope(new SetInstanceOverridesCommand(6, new Dictionary { [requestedName] = "42" }), "Deployer")); + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + + // Feed what the management path persisted into the real flattener. + foreach (var o in stored) instance.AttributeOverrides.Add(o); + var flattened = new ZB.MOM.WW.ScadaBridge.TemplateEngine.Flattening.FlatteningService().Flatten( + instance, [template], + new Dictionary>(), + new Dictionary>(), + new Dictionary()); + Assert.True(flattened.IsSuccess, flattened.IsFailure ? flattened.Error : null); + foreach (var o in stored) + Assert.True( + flattened.Value.Attributes.Any(a => a.CanonicalName == o.AttributeName && a.Source == "Override"), + $"Override '{o.AttributeName}' was stored by the management path but the flattener dropped it as unresolved."); + + var setpoint = Assert.Single(flattened.Value.Attributes, a => a.CanonicalName == "Setpoint"); + if (accepted) + { + Assert.IsType(response); + Assert.Equal("42", setpoint.Value); + Assert.Equal("Override", setpoint.Source); + } + else + { + var error = Assert.IsType(response); + Assert.Contains("did you mean 'Setpoint'", error.Error); + Assert.Empty(stored); + Assert.Equal("0", setpoint.Value); + } + } + /// ScadaBridge#105: a bulk native-source override differing only by case is rejected, nothing stored. [Fact] public void SetInstanceNativeAlarmSourceOverrides_CaseVariantName_IsRejectedWithHint() diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Services/InstanceServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Services/InstanceServiceTests.cs index 1ea7a251..4789ae02 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Services/InstanceServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Services/InstanceServiceTests.cs @@ -133,6 +133,74 @@ public class InstanceServiceTests Assert.Equal("99", result.Value.OverrideValue); } + /// + /// ScadaBridge#105, end to end for the single-attribute path the Central UI uses + /// (): whatever it stores must be + /// applied by the real flattener. The exact-case row is the positive control; a name + /// differing from the member only by case is refused with a hint and nothing is stored. + /// + /// The override name the caller sends. + /// Whether the write is expected to succeed. + /// A task that represents the asynchronous operation. + [Theory] + [InlineData("Threshold", true)] + [InlineData("threshold", false)] + [InlineData("THRESHOLD", false)] + public async Task SetAttributeOverride_EveryStoredOverride_IsAppliedByTheFlattener(string requestedName, bool accepted) + { + var template = new Template("Base") { Id = 1 }; + template.Attributes.Add(new TemplateAttribute("Threshold") { TemplateId = 1, DataType = DataType.Double, Value = "50.0" }); + var instance = new Instance("Inst1") { Id = 1, TemplateId = 1, SiteId = 1 }; + _repoMock.Setup(r => r.GetInstanceByIdAsync(1, It.IsAny())).ReturnsAsync(instance); + _repoMock.Setup(r => r.GetAttributesByTemplateIdAsync(1, It.IsAny())) + .ReturnsAsync(template.Attributes.ToList()); + _repoMock.Setup(r => r.GetOverridesByInstanceIdAsync(1, It.IsAny())) + .ReturnsAsync(new List()); + _repoMock.Setup(r => r.SaveChangesAsync(It.IsAny())).ReturnsAsync(1); + var stored = new List(); + _repoMock.Setup(r => r.AddInstanceAttributeOverrideAsync(It.IsAny(), It.IsAny())) + .Callback((o, _) => stored.Add(o)) + .Returns(Task.CompletedTask); + + var result = await _sut.SetAttributeOverrideAsync(1, requestedName, "75.0", "admin"); + + foreach (var o in stored) instance.AttributeOverrides.Add(o); + var flattened = new ZB.MOM.WW.ScadaBridge.TemplateEngine.Flattening.FlatteningService().Flatten( + instance, [template], + new Dictionary>(), + new Dictionary>(), + new Dictionary()); + Assert.True(flattened.IsSuccess); + foreach (var o in stored) + Assert.True( + flattened.Value.Attributes.Any(a => a.CanonicalName == o.AttributeName && a.Source == "Override"), + $"Override '{o.AttributeName}' was stored but the flattener dropped it as unresolved."); + + var threshold = Assert.Single(flattened.Value.Attributes, a => a.CanonicalName == "Threshold"); + if (accepted) + { + Assert.True(result.IsSuccess); + Assert.Equal("75.0", threshold.Value); + Assert.Equal("Override", threshold.Source); + } + else + { + Assert.True(result.IsFailure); + Assert.Contains("did you mean 'Threshold'", result.Error); + Assert.Empty(stored); + Assert.Equal("50.0", threshold.Value); + } + } + + /// ScadaBridge#105: an ambiguous case-insensitive match (two candidates differing only by case) gets no hint. + [Fact] + public void CaseMismatchHint_NamesOnlyAnUnambiguousMatch() + { + Assert.Contains("did you mean 'Setpoint'", OverrideNameMatching.CaseMismatchHint("setpoint", ["Setpoint", "Other"])); + Assert.Equal(string.Empty, OverrideNameMatching.CaseMismatchHint("setpoint", ["Setpoint", "SETPOINT"])); + Assert.Equal(string.Empty, OverrideNameMatching.CaseMismatchHint("missing", ["Setpoint"])); + } + // --- SetAlarmOverrideAsync validation regression --- private static Template TemplateWithAlarms(int id, params TemplateAlarm[] alarms)