From ded4c417985107ce4cc4e8f7c61b6e7e64a3179d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 14:31:36 -0400 Subject: [PATCH] fix(mqtt): key tag definitions by RawPath, not the TagConfig blob Task 2 review follow-up. The plan specified MqttEquipmentTagParser.TryParse(reference) with def.Name = the TagConfig blob, and told us to mirror a type named ModbusEquipmentTagParser. Both are plan defects: - EquipmentTagRefResolver documents that a v3 driver reference "is now always a RawPath" and that the blob-parse fallback is retired. Keying Name by the blob would make OnDataChange publish under a reference that never matches the RawPath-keyed fan-out in DriverHostActor - silently dead in production with every unit test still green. - ModbusEquipmentTagParser does not exist. The six sibling drivers all use TagDefinitionFactory.FromTagConfig(tagConfig, rawPath, out def). Changes: - Rename MqttEquipmentTagParser -> MqttTagDefinitionFactory; TryParse(reference, out def) -> FromTagConfig(tagConfig, rawPath, out def) setting Name: rawPath, matching ModbusTagDefinitionFactory's structure, param docs and guard order. - Pin the identity contract with a dedicated test so a regression to blob-keying goes red. - Read qos with the same strictness as the enums: a present-but-invalid qos ("high" / 1.5 / 5 / null) now rejects the tag and is warned by Inspect, instead of being silently absorbed into the driver-level default and handing the operator a weaker delivery guarantee than the one they authored. - No ToTagConfig inverse: the siblings carry one solely for their Driver..Cli project, the MQTT plan defines none, and the AdminUI editor template references no driver factory. Recorded as an explicit YAGNI call in the type doc rather than added speculatively. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- .../MqttEquipmentTagParser.cs | 164 ------------- .../MqttTagDefinition.cs | 17 +- .../MqttTagDefinitionFactory.cs | 198 ++++++++++++++++ .../MqttEquipmentTagParserTests.cs | 143 ----------- .../MqttTagDefinitionFactoryTests.cs | 224 ++++++++++++++++++ 5 files changed, 432 insertions(+), 314 deletions(-) delete mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttEquipmentTagParser.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs delete mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttEquipmentTagParserTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttTagDefinitionFactoryTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttEquipmentTagParser.cs deleted file mode 100644 index 24e33600..00000000 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttEquipmentTagParser.cs +++ /dev/null @@ -1,164 +0,0 @@ -using System.Diagnostics.CodeAnalysis; -using System.Text.Json; -using ZB.MOM.WW.OtOpcUa.Core.Abstractions; - -namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; - -/// -/// Turns an authored equipment tag's opaque wire reference — its TagConfig JSON blob -/// — into a typed . Mirrors ModbusTagDefinitionFactory: a -/// leading { marks a TagConfig blob, enum fields are read STRICTLY, and the two entry -/// points carry deliberately different strictness contracts. -/// -/// -/// -/// is the runtime path. It never throws; anything -/// malformed returns so the driver surfaces -/// BadNodeIdUnknown rather than a misleading Good off a wrong-typed -/// default. -/// -/// -/// -/// -/// is the deploy-time path. It returns human-readable -/// warnings so a bad tag config surfaces at deploy instead of silently going dark at -/// runtime. -/// -/// -/// -/// -/// Sparkplug-descriptor parsing (groupId / edgeNodeId / deviceId / -/// metricName) is a deliberate P1 stub — the fields exist on -/// but are never populated here until the P2 tasks fill them. -/// -/// -public static class MqttEquipmentTagParser -{ - /// The JSONPath applied when a Json-format blob omits jsonPath: the document root. - private const string RootJsonPath = "$"; - - /// The MQTT wildcard characters — illegal in a tag's (concrete) subscription topic. - private static readonly char[] TopicWildcards = ['+', '#']; - - /// - /// Parses a driver wire into a typed definition. Returns - /// — never throws — for a null/blank reference, a reference that is not - /// a TagConfig blob (no leading {), unparseable JSON, a missing or blank topic, a - /// qos outside 0–2, or a present-but-invalid (typo'd) payloadFormat / - /// dataType. The produced definition's is - /// itself, which is the key published values are routed on. - /// - /// A wildcard topic (+ / #) is deliberately NOT rejected here: it is - /// ambiguous rather than unparseable, and is the operator-visible - /// surface for it. Rejecting it at runtime would turn an authoring mistake into a silent - /// BadNodeIdUnknown with no stated cause. - /// - /// - /// The tag's wire reference (its authored TagConfig JSON). - /// The parsed definition when this returns ; otherwise . - /// when is a valid MQTT tag config. - public static bool TryParse(string reference, [NotNullWhen(true)] out MqttTagDefinition? def) - { - def = null; - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return false; - try - { - using var doc = JsonDocument.Parse(reference); - var root = doc.RootElement; - if (root.ValueKind != JsonValueKind.Object) return false; - - // topic is the whole point of a Plain-mode tag — without one there is nothing to subscribe - // to, so an absent/blank value is a hard reject rather than a defaulted empty subscription. - var topic = ReadString(root, "topic"); - if (string.IsNullOrWhiteSpace(topic)) return false; - - // Strict enum reads: a present-but-invalid (typo'd) value rejects the whole tag - // (→ BadNodeIdUnknown) instead of silently defaulting to a wrong-typed Good. - if (!TagConfigJson.TryReadEnumStrict(root, "payloadFormat", MqttPayloadFormat.Json, out var payloadFormat)) - return false; - if (!TagConfigJson.TryReadEnumStrict(root, "dataType", DriverDataType.String, out var dataType)) - return false; - - // qos is an MQTT protocol field with exactly three legal values; anything else would be - // rejected by the broker at SUBSCRIBE time, so reject it here where the cause is visible. - var qos = ReadOptionalInt(root, "qos"); - if (qos is { } q && q is < 0 or > 2) return false; - - var jsonPath = ReadString(root, "jsonPath"); - if (string.IsNullOrEmpty(jsonPath)) jsonPath = RootJsonPath; - - def = new MqttTagDefinition( - Name: reference, - Topic: topic, - PayloadFormat: payloadFormat, - JsonPath: jsonPath, - DataType: dataType, - Qos: qos, - RetainSeed: ReadBoolOrDefault(root, "retainSeed", defaultValue: true)); - return true; - } - catch (JsonException) { return false; } - catch (FormatException) { return false; } - catch (InvalidOperationException) { return false; } - } - - /// - /// Deploy-time inspection of an equipment-tag TagConfig blob. Returns human-readable - /// warnings for a structurally unparseable blob (which the runtime turns into a silent - /// BadNodeIdUnknown), for present-but-invalid payloadFormat / dataType - /// enum values, and for a wildcard tag topic (a tag bound to + / # would - /// receive values from many topics — ambiguous, and almost never what the operator meant). - /// Empty when the blob is clean or is not a TagConfig object at all. Never throws. - /// - /// The equipment tag's TagConfig JSON. - /// The warnings; empty when clean. - public static IReadOnlyList Inspect(string reference) - { - var warnings = new List(); - if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; - try - { - using var doc = JsonDocument.Parse(reference); - var root = doc.RootElement; - if (root.ValueKind != JsonValueKind.Object) - { - warnings.Add("Mqtt TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown)."); - return warnings; - } - foreach (var w in new[] - { - TagConfigJson.DescribeInvalidEnum(root, "payloadFormat"), - TagConfigJson.DescribeInvalidEnum(root, "dataType"), - }) - { - if (w is not null) warnings.Add(w); - } - var topic = ReadString(root, "topic"); - if (topic.IndexOfAny(TopicWildcards) >= 0) - { - warnings.Add( - $"topic '{topic}' contains an MQTT wildcard (+ or #); a tag's topic must be concrete, " + - "or the tag will be fed by every matching topic."); - } - } - catch (JsonException) - { - warnings.Add("Mqtt TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown)."); - } - return warnings; - } - - private static string ReadString(JsonElement o, string name) - => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.String - ? e.GetString() ?? "" - : ""; - - private static int? ReadOptionalInt(JsonElement o, string name) - => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.Number - && e.TryGetInt32(out var v) ? v : null; - - private static bool ReadBoolOrDefault(JsonElement o, string name, bool defaultValue) - => o.TryGetProperty(name, out var e) && e.ValueKind is JsonValueKind.True or JsonValueKind.False - ? e.GetBoolean() - : defaultValue; -} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinition.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinition.cs index 0d139be5..d4f6663b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinition.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinition.cs @@ -3,10 +3,10 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; /// -/// The driver's internal per-tag descriptor — the parsed form of an authored equipment tag's -/// TagConfig JSON, produced by and looked up -/// through the shared exactly as Modbus does. See -/// docs/plans/2026-07-15-mqtt-sparkplug-driver-design.md §5.2 / §5.3. +/// The driver's internal per-tag descriptor — the parsed form of an authored raw tag's +/// TagConfig JSON, produced by and +/// looked up through the shared exactly as Modbus does. +/// See docs/plans/2026-07-15-mqtt-sparkplug-driver-design.md §5.2 / §5.3. /// /// The record carries both ingest shapes: the Plain-MQTT fields /// ( … ), populated in P1, and the Sparkplug B @@ -18,9 +18,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; /// /// /// -/// The definition's identity: the exact wire reference string the driver was handed -/// (the tag's authored TagConfig blob). Published values key the forward router on this, -/// so it must round-trip byte-for-byte. +/// The definition's identity: the tag's RawPath — the cluster-scoped slash path that is the +/// v3 driver wire reference. This is the key resolves +/// on, the key OnDataChange must publish under, and the key DriverHostActor fans out +/// to the raw + UNS NodeIds. It is emphatically not the authored TagConfig blob: the +/// pre-v3 blob-as-reference shape is retired, and publishing under a blob key would silently miss +/// the RawPath-keyed fan-out while every unit test still passed. /// /// The concrete MQTT topic this tag subscribes to (Plain mode). /// How the received payload is decoded (Plain mode). diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs new file mode 100644 index 00000000..697256b1 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttTagDefinitionFactory.cs @@ -0,0 +1,198 @@ +using System.Text.Json; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; + +/// +/// v3 pure mapper: turns an authored raw tag's TagConfig JSON (the shape produced by the +/// AdminUI MqttTagConfigModel) into an . Under v3 a tag's +/// identity is its RawPath (a cluster-scoped slash path), not the address blob — so the +/// produced definition's is the RawPath the driver was handed, +/// which is exactly the wire reference the driver's RawPath → def resolver keys on. The driver +/// builds that table by mapping each the deploy artifact delivers through +/// . +/// +/// Two entry points carry deliberately different strictness contracts. +/// is the runtime path: it never throws, and anything +/// malformed returns so the driver surfaces BadNodeIdUnknown +/// rather than a misleading Good off a wrong-typed default. is the +/// deploy-time path: it returns human-readable warnings so a bad tag config surfaces at +/// deploy instead of silently going dark at runtime. +/// +/// +/// Sparkplug-descriptor parsing (groupId / edgeNodeId / deviceId / +/// metricName) is a deliberate P1 stub — the fields exist on +/// but are never populated here until the P2 tasks fill them. +/// +/// +/// No ToTagConfig inverse — a deliberate YAGNI call. The six sibling factories +/// each carry one solely to serve their Driver.<X>.Cli project, which synthesises +/// blobs from operator flags; the MQTT plan defines no +/// Mqtt.Cli. The AdminUI typed editor does not need one either — the +/// <Driver>TagConfigModel template serialises through its own preserved +/// JsonObject key bag and references no driver factory (verified: no +/// TagDefinitionFactory reference exists anywhere under the AdminUI project). Add the +/// inverse when a real caller appears, not before. +/// +/// +public static class MqttTagDefinitionFactory +{ + /// The JSONPath applied when a Json-format blob omits jsonPath: the document root. + private const string RootJsonPath = "$"; + + /// The MQTT wildcard characters — illegal in a tag's (concrete) subscription topic. + private static readonly char[] TopicWildcards = ['+', '#']; + + /// + /// Maps an authored TagConfig object to a typed definition keyed by . + /// The input is always an authored TagConfig JSON object (there is no name-vs-blob heuristic in v3). + /// Enum fields (payloadFormat / dataType) and qos are read STRICTLY — a + /// present-but-invalid (typo'd) value rejects the whole tag (returns ⇒ the + /// driver surfaces BadNodeIdUnknown) rather than silently defaulting to a wrong-typed Good or + /// a downgraded delivery guarantee. Never throws. + /// + /// A wildcard topic (+ / #) is deliberately NOT rejected here: it is + /// ambiguous rather than unparseable, and is the operator-visible surface + /// for it. Rejecting it at runtime would turn an authoring mistake into a silent + /// BadNodeIdUnknown with no stated cause. + /// + /// + /// The authored equipment-tag TagConfig JSON. + /// The tag's RawPath — becomes the definition's identity (Name). + /// The mapped definition when this returns . + /// when is a valid MQTT tag-config object. + public static bool FromTagConfig(string tagConfig, string rawPath, out MqttTagDefinition def) + { + def = null!; + if (string.IsNullOrWhiteSpace(tagConfig) || tagConfig[0] != '{') return false; + try + { + using var doc = JsonDocument.Parse(tagConfig); + var root = doc.RootElement; + if (root.ValueKind != JsonValueKind.Object) return false; + + // topic is the whole point of a Plain-mode tag — without one there is nothing to subscribe + // to, so an absent/blank value is a hard reject rather than a defaulted empty subscription. + var topic = ReadString(root, "topic"); + if (string.IsNullOrWhiteSpace(topic)) return false; + + // Strict enum reads: a present-but-invalid (typo'd) value rejects the whole tag + // (→ BadNodeIdUnknown) instead of silently defaulting to a wrong-typed Good. + if (!TagConfigJson.TryReadEnumStrict(root, "payloadFormat", MqttPayloadFormat.Json, out var payloadFormat)) + return false; + if (!TagConfigJson.TryReadEnumStrict(root, "dataType", DriverDataType.String, out var dataType)) + return false; + + // qos is read with the same strictness, for the same reason: silently absorbing a malformed + // "qos":"high" / "qos":1.5 / "qos":5 into the driver-level default would hand the operator a + // WEAKER delivery guarantee than the one they authored, with nothing to show for it. + if (!TryReadQosStrict(root, out var qos)) return false; + + var jsonPath = ReadString(root, "jsonPath"); + if (string.IsNullOrEmpty(jsonPath)) jsonPath = RootJsonPath; + + def = new MqttTagDefinition( + Name: rawPath, + Topic: topic, + PayloadFormat: payloadFormat, + JsonPath: jsonPath, + DataType: dataType, + Qos: qos, + RetainSeed: ReadBoolOrDefault(root, "retainSeed", defaultValue: true)); + return true; + } + catch (JsonException) { return false; } + catch (FormatException) { return false; } + catch (InvalidOperationException) { return false; } + } + + /// + /// Deploy-time inspection of an equipment-tag TagConfig blob. Returns human-readable + /// warnings for a structurally unparseable blob (which the runtime turns into a silent + /// BadNodeIdUnknown), for present-but-invalid payloadFormat / dataType / + /// qos values, and for a wildcard tag topic (a tag bound to + / # + /// would receive values from many topics — ambiguous, and almost never what the operator meant). + /// Empty when the blob is clean or is not an equipment-tag TagConfig object. Never throws. + /// + /// The equipment tag's TagConfig JSON. + /// The warnings; empty when clean. + public static IReadOnlyList Inspect(string reference) + { + var warnings = new List(); + if (string.IsNullOrWhiteSpace(reference) || reference[0] != '{') return warnings; + try + { + using var doc = JsonDocument.Parse(reference); + var root = doc.RootElement; + if (root.ValueKind != JsonValueKind.Object) + { + warnings.Add("Mqtt TagConfig root is not a JSON object — the tag will not resolve (BadNodeIdUnknown)."); + return warnings; + } + foreach (var w in new[] + { + TagConfigJson.DescribeInvalidEnum(root, "payloadFormat"), + TagConfigJson.DescribeInvalidEnum(root, "dataType"), + DescribeInvalidQos(root), + }) + { + if (w is not null) warnings.Add(w); + } + var topic = ReadString(root, "topic"); + if (topic.IndexOfAny(TopicWildcards) >= 0) + { + warnings.Add( + $"value '{topic}' for 'topic' contains an MQTT wildcard (+ or #); a tag's topic must be " + + "concrete, or the tag will be fed by every matching topic."); + } + } + catch (JsonException) + { + warnings.Add("Mqtt TagConfig is not valid JSON — the tag will not resolve (BadNodeIdUnknown)."); + } + return warnings; + } + + /// + /// Strict qos read, mirroring 's + /// absent / valid / present-but-invalid split: absent ⇒ (the driver-level + /// wins); a JSON integer in 0–2 ⇒ that value; anything + /// else present (non-number, non-integer, or out of range) ⇒ the read FAILS. + /// + /// The TagConfig root object. + /// The parsed QoS, or when the field is absent. + /// only when qos is present but not a legal MQTT QoS. + private static bool TryReadQosStrict(JsonElement o, out int? qos) + { + qos = null; + if (!o.TryGetProperty("qos", out var e)) return true; // absent + if (e.ValueKind != JsonValueKind.Number || !e.TryGetInt32(out var v)) return false; + if (v is < 0 or > 2) return false; + qos = v; + return true; + } + + /// + /// A human-readable warning for a present-but-invalid qos field, or + /// when it is absent or valid — the qos counterpart of + /// . + /// + /// The TagConfig root object. + /// The warning text, or . + private static string? DescribeInvalidQos(JsonElement o) + { + if (!o.TryGetProperty("qos", out var e)) return null; + if (e.ValueKind == JsonValueKind.Number && e.TryGetInt32(out var v) && v is >= 0 and <= 2) return null; + return $"value '{e.GetRawText()}' for 'qos' is not a valid MQTT QoS; valid: 0, 1, 2"; + } + + private static string ReadString(JsonElement o, string name) + => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.String + ? e.GetString() ?? "" + : ""; + + private static bool ReadBoolOrDefault(JsonElement o, string name, bool defaultValue) + => o.TryGetProperty(name, out var e) && e.ValueKind is JsonValueKind.True or JsonValueKind.False + ? e.GetBoolean() + : defaultValue; +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttEquipmentTagParserTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttEquipmentTagParserTests.cs deleted file mode 100644 index 037f7b2a..00000000 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttEquipmentTagParserTests.cs +++ /dev/null @@ -1,143 +0,0 @@ -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Core.Abstractions; -using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; - -namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; - -/// -/// Covers the two entry points of and their deliberately -/// different strictness contracts: TryParse is the runtime path (never throws, a -/// malformed blob is a hard reject that upstream turns into BadNodeIdUnknown) and -/// Inspect is the deploy-time path (human-readable warnings so a bad tag surfaces at -/// deploy instead of going dark at runtime). -/// -public sealed class MqttEquipmentTagParserTests -{ - [Fact] - public void TryParse_PlainJsonBlob_PopulatesTopicAndPath() - { - // NOTE: the plan's sample blob wrote "dataType":"Double"; the authoritative type is - // DriverDataType (design §6.1), whose 64-bit float member is Float64. "Double" is - // therefore a typo the strict read rejects — pinned by TryParse_TypoedDataType_RejectsStrict. - const string r = """{"topic":"factory/oven/temp","payloadFormat":"Json","jsonPath":"$.value","dataType":"Float64","qos":1}"""; - MqttEquipmentTagParser.TryParse(r, out var def).ShouldBeTrue(); - def!.Topic.ShouldBe("factory/oven/temp"); - def.PayloadFormat.ShouldBe(MqttPayloadFormat.Json); - def.JsonPath.ShouldBe("$.value"); - def.DataType.ShouldBe(DriverDataType.Float64); - def.Qos.ShouldBe(1); - def.Name.ShouldBe(r); // the def Name == reference string (forward-router key) - } - - [Fact] - public void TryParse_TypoedPayloadFormat_RejectsStrict() - => MqttEquipmentTagParser.TryParse( - """{"topic":"a/b","payloadFormat":"Jason","dataType":"Float64"}""", out _).ShouldBeFalse(); - - [Fact] - public void TryParse_TypoedDataType_RejectsStrict() - => MqttEquipmentTagParser.TryParse( - """{"topic":"a/b","payloadFormat":"Json","dataType":"Double"}""", out _).ShouldBeFalse(); - - [Fact] - public void Inspect_WildcardTopic_ReturnsWarning() - { - // The blob is otherwise clean (payloadFormat parses, dataType absent), so the wildcard check - // is the ONLY thing that can produce a warning here — the assertion cannot pass vacuously. - var warnings = MqttEquipmentTagParser.Inspect("""{"topic":"a/+/c","payloadFormat":"Raw"}"""); - warnings.ShouldNotBeEmpty(); - warnings.ShouldHaveSingleItem().ShouldContain("wildcard", Case.Insensitive); - } - - [Theory] - [InlineData("a/#")] - [InlineData("+/b/c")] - [InlineData("a/+/c")] - public void Inspect_EachWildcardForm_Warns(string topic) - => MqttEquipmentTagParser.Inspect($$"""{"topic":"{{topic}}","payloadFormat":"Raw"}""").ShouldNotBeEmpty(); - - [Fact] - public void TryParse_WildcardTopic_StillParses_WarningIsDeployTimeOnly() - { - // A wildcard tag topic is ambiguous, not unparseable: the deploy-time Inspect pass is the - // designed surface for it. Rejecting it at runtime would turn an authoring mistake into a - // silent BadNodeIdUnknown with no operator-visible cause. - MqttEquipmentTagParser.TryParse("""{"topic":"a/+/c","payloadFormat":"Raw"}""", out var def).ShouldBeTrue(); - def!.Topic.ShouldBe("a/+/c"); - } - - [Fact] - public void TryParse_RawFormat_AppliesDefaults() - { - const string r = """{"topic":"factory/oven/blob","payloadFormat":"Raw"}"""; - MqttEquipmentTagParser.TryParse(r, out var def).ShouldBeTrue(); - def!.PayloadFormat.ShouldBe(MqttPayloadFormat.Raw); - def.JsonPath.ShouldBe("$"); // absent ⇒ the root default - def.Qos.ShouldBeNull(); // absent ⇒ the driver-level DefaultQos wins - def.RetainSeed.ShouldBeTrue(); // absent ⇒ seed from the retained message - def.DataType.ShouldBe(DriverDataType.String); // absent ⇒ the documented fallback - } - - [Fact] - public void TryParse_ExplicitRetainSeedFalse_IsHonoured() - { - MqttEquipmentTagParser.TryParse( - """{"topic":"a/b","payloadFormat":"Raw","retainSeed":false}""", out var def).ShouldBeTrue(); - def!.RetainSeed.ShouldBeFalse(); - } - - [Fact] - public void TryParse_PlainBlob_LeavesSparkplugDescriptorFieldsNull() - { - // P2 (Tasks 15–26) fills these; a plain-mode blob must leave them unset so a future - // mode discriminator cannot mistake a plain tag for a Sparkplug one. - MqttEquipmentTagParser.TryParse( - """{"topic":"a/b","payloadFormat":"Raw"}""", out var def).ShouldBeTrue(); - def!.GroupId.ShouldBeNull(); - def.EdgeNodeId.ShouldBeNull(); - def.DeviceId.ShouldBeNull(); - def.MetricName.ShouldBeNull(); - } - - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - [InlineData("factory/oven/temp")] // a bare reference, not a TagConfig blob (no leading '{') - [InlineData("[1,2,3]")] // valid JSON, wrong root kind - [InlineData("{ not json at all")] // unparseable — must not throw - [InlineData("""{"payloadFormat":"Raw"}""")] // no topic ⇒ nothing to subscribe to - [InlineData("""{"topic":"","payloadFormat":"Raw"}""")] // blank topic - [InlineData("""{"topic":"a/b","qos":5}""")] // QoS outside 0–2 is an illegal subscription - public void TryParse_MalformedReference_ReturnsFalseAndNeverThrows(string? reference) - => MqttEquipmentTagParser.TryParse(reference!, out _).ShouldBeFalse(); - - [Fact] - public void Inspect_CleanBlob_ReturnsEmpty() - => MqttEquipmentTagParser.Inspect( - """{"topic":"a/b","payloadFormat":"Json","jsonPath":"$.v","dataType":"Float64"}""").ShouldBeEmpty(); - - [Fact] - public void Inspect_TypoedEnums_WarnsPerField() - { - var warnings = MqttEquipmentTagParser.Inspect( - """{"topic":"a/b","payloadFormat":"Jason","dataType":"Double"}"""); - warnings.Count.ShouldBe(2); - warnings.ShouldContain(w => w.Contains("payloadFormat", StringComparison.Ordinal)); - warnings.ShouldContain(w => w.Contains("dataType", StringComparison.Ordinal)); - } - - [Fact] - public void Inspect_UnparseableBlob_Warns() - => MqttEquipmentTagParser.Inspect("{ not json at all").ShouldNotBeEmpty(); - - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - [InlineData("factory/oven/temp")] - [InlineData("[1,2,3]")] // no leading '{' ⇒ not a TagConfig blob at all (mirrors Modbus) - public void Inspect_NotATagConfigBlob_ReturnsEmpty(string? reference) - => MqttEquipmentTagParser.Inspect(reference!).ShouldBeEmpty(); -} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttTagDefinitionFactoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttTagDefinitionFactoryTests.cs new file mode 100644 index 00000000..6bd57140 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttTagDefinitionFactoryTests.cs @@ -0,0 +1,224 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; + +/// +/// Covers the two entry points of and their deliberately +/// different strictness contracts: FromTagConfig is the runtime path (never throws, a +/// malformed blob is a hard reject that upstream turns into BadNodeIdUnknown) and +/// Inspect is the deploy-time path (human-readable warnings so a bad tag surfaces at +/// deploy instead of going dark at runtime). +/// +public sealed class MqttTagDefinitionFactoryTests +{ + private const string RawPath = "Plant/Mqtt/oven/Temp"; + + [Fact] + public void FromTagConfig_PlainJsonBlob_PopulatesTopicAndPath() + { + // NOTE: the plan's sample blob wrote "dataType":"Double"; the authoritative type is + // DriverDataType (design §6.1), whose 64-bit float member is Float64. "Double" is therefore a + // typo the strict read rejects — pinned by FromTagConfig_TypoedDataType_RejectsStrict. + const string blob = """{"topic":"factory/oven/temp","payloadFormat":"Json","jsonPath":"$.value","dataType":"Float64","qos":1}"""; + MqttTagDefinitionFactory.FromTagConfig(blob, RawPath, out var def).ShouldBeTrue(); + def.Topic.ShouldBe("factory/oven/temp"); + def.PayloadFormat.ShouldBe(MqttPayloadFormat.Json); + def.JsonPath.ShouldBe("$.value"); + def.DataType.ShouldBe(DriverDataType.Float64); + def.Qos.ShouldBe(1); + } + + /// + /// The v3 identity contract, stated explicitly so a regression back to blob-keying goes red. + /// MUST be the RawPath the driver was handed — it is the key + /// EquipmentTagRefResolver looks up, the key OnDataChange publishes under, and the + /// key DriverHostActor fans out to the raw + UNS NodeIds. Keying it by the TagConfig blob + /// (the pre-v3, now-retired shape) would leave the driver silently dead in production while every + /// other test here still passed. + /// + [Fact] + public void FromTagConfig_DefinitionIdentity_IsTheRawPath_NotTheBlob() + { + const string blob = """{"topic":"factory/oven/temp","payloadFormat":"Raw"}"""; + MqttTagDefinitionFactory.FromTagConfig(blob, RawPath, out var def).ShouldBeTrue(); + def.Name.ShouldBe(RawPath); + def.Name.ShouldNotBe(blob); + } + + /// + /// The identity is whatever RawPath the caller supplies — the factory must never re-derive it + /// from the blob's contents (e.g. from topic), or two tags sharing a topic would collide. + /// + [Theory] + [InlineData("Plant/Mqtt/oven/Temp")] + [InlineData("SiteA/Mqtt/line3/Oven/Setpoint")] + public void FromTagConfig_UsesSuppliedRawPathVerbatim(string rawPath) + { + MqttTagDefinitionFactory.FromTagConfig( + """{"topic":"shared/topic","payloadFormat":"Raw"}""", rawPath, out var def).ShouldBeTrue(); + def.Name.ShouldBe(rawPath); + } + + [Fact] + public void FromTagConfig_TypoedPayloadFormat_RejectsStrict() + => MqttTagDefinitionFactory.FromTagConfig( + """{"topic":"a/b","payloadFormat":"Jason","dataType":"Float64"}""", RawPath, out _).ShouldBeFalse(); + + [Fact] + public void FromTagConfig_TypoedDataType_RejectsStrict() + => MqttTagDefinitionFactory.FromTagConfig( + """{"topic":"a/b","payloadFormat":"Json","dataType":"Double"}""", RawPath, out _).ShouldBeFalse(); + + [Fact] + public void Inspect_WildcardTopic_ReturnsWarning() + { + // The blob is otherwise clean (payloadFormat parses, dataType + qos absent), so the wildcard + // check is the ONLY thing that can produce a warning — this cannot pass vacuously. + var warnings = MqttTagDefinitionFactory.Inspect("""{"topic":"a/+/c","payloadFormat":"Raw"}"""); + warnings.ShouldNotBeEmpty(); + warnings.ShouldHaveSingleItem().ShouldContain("wildcard", Case.Insensitive); + } + + [Theory] + [InlineData("a/#")] + [InlineData("+/b/c")] + [InlineData("a/+/c")] + public void Inspect_EachWildcardForm_Warns(string topic) + => MqttTagDefinitionFactory.Inspect($$"""{"topic":"{{topic}}","payloadFormat":"Raw"}""").ShouldNotBeEmpty(); + + [Fact] + public void FromTagConfig_WildcardTopic_StillParses_WarningIsDeployTimeOnly() + { + // A wildcard tag topic is ambiguous, not unparseable: the deploy-time Inspect pass is the + // designed surface for it. Rejecting it at runtime would turn an authoring mistake into a + // silent BadNodeIdUnknown with no operator-visible cause. + MqttTagDefinitionFactory.FromTagConfig( + """{"topic":"a/+/c","payloadFormat":"Raw"}""", RawPath, out var def).ShouldBeTrue(); + def.Topic.ShouldBe("a/+/c"); + } + + [Fact] + public void FromTagConfig_RawFormat_AppliesDefaults() + { + MqttTagDefinitionFactory.FromTagConfig( + """{"topic":"factory/oven/blob","payloadFormat":"Raw"}""", RawPath, out var def).ShouldBeTrue(); + def.PayloadFormat.ShouldBe(MqttPayloadFormat.Raw); + def.JsonPath.ShouldBe("$"); // absent ⇒ the root default + def.Qos.ShouldBeNull(); // absent ⇒ the driver-level DefaultQos wins + def.RetainSeed.ShouldBeTrue(); // absent ⇒ seed from the retained message + def.DataType.ShouldBe(DriverDataType.String); // absent ⇒ the documented fallback + } + + [Fact] + public void FromTagConfig_ExplicitRetainSeedFalse_IsHonoured() + { + MqttTagDefinitionFactory.FromTagConfig( + """{"topic":"a/b","payloadFormat":"Raw","retainSeed":false}""", RawPath, out var def).ShouldBeTrue(); + def.RetainSeed.ShouldBeFalse(); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + public void FromTagConfig_EachLegalQos_IsAccepted(int qos) + { + MqttTagDefinitionFactory.FromTagConfig( + $$"""{"topic":"a/b","payloadFormat":"Raw","qos":{{qos}}}""", RawPath, out var def).ShouldBeTrue(); + def.Qos.ShouldBe(qos); + } + + [Fact] + public void FromTagConfig_PlainBlob_LeavesSparkplugDescriptorFieldsNull() + { + // P2 (Tasks 15–26) fills these; a plain-mode blob must leave them unset so a future + // mode discriminator cannot mistake a plain tag for a Sparkplug one. + MqttTagDefinitionFactory.FromTagConfig( + """{"topic":"a/b","payloadFormat":"Raw"}""", RawPath, out var def).ShouldBeTrue(); + def.GroupId.ShouldBeNull(); + def.EdgeNodeId.ShouldBeNull(); + def.DeviceId.ShouldBeNull(); + def.MetricName.ShouldBeNull(); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("factory/oven/temp")] // a bare reference, not a TagConfig blob (no leading '{') + [InlineData("[1,2,3]")] // valid JSON, wrong root kind + [InlineData("{ not json at all")] // unparseable — must not throw + [InlineData("""{"payloadFormat":"Raw"}""")] // no topic ⇒ nothing to subscribe to + [InlineData("""{"topic":"","payloadFormat":"Raw"}""")] // blank topic + [InlineData("""{"topic":" ","payloadFormat":"Raw"}""")] // whitespace-only topic + public void FromTagConfig_MalformedBlob_ReturnsFalseAndNeverThrows(string? tagConfig) + => MqttTagDefinitionFactory.FromTagConfig(tagConfig!, RawPath, out _).ShouldBeFalse(); + + /// + /// A present-but-invalid qos is rejected in every malformed shape, not just the + /// out-of-range numeric one. Silently absorbing "high" / 1.5 into the driver-level + /// default would hand the operator a WEAKER delivery guarantee than the one they authored. + /// + [Theory] + [InlineData("5")] // out of range + [InlineData("-1")] // out of range + [InlineData("\"high\"")] // wrong JSON type + [InlineData("\"1\"")] // stringly-typed number + [InlineData("1.5")] // non-integer + [InlineData("true")] // wrong JSON type + [InlineData("null")] // an explicit null is not "absent" + public void FromTagConfig_MalformedQos_RejectsStrict(string qosToken) + => MqttTagDefinitionFactory.FromTagConfig( + $$"""{"topic":"a/b","payloadFormat":"Raw","qos":{{qosToken}}}""", RawPath, out _).ShouldBeFalse(); + + [Fact] + public void Inspect_CleanBlob_ReturnsEmpty() + => MqttTagDefinitionFactory.Inspect( + """{"topic":"a/b","payloadFormat":"Json","jsonPath":"$.v","dataType":"Float64","qos":2}""").ShouldBeEmpty(); + + [Fact] + public void Inspect_TypoedEnums_WarnsPerField() + { + var warnings = MqttTagDefinitionFactory.Inspect( + """{"topic":"a/b","payloadFormat":"Jason","dataType":"Double"}"""); + warnings.Count.ShouldBe(2); + warnings.ShouldContain(w => w.Contains("payloadFormat", StringComparison.Ordinal)); + warnings.ShouldContain(w => w.Contains("dataType", StringComparison.Ordinal)); + } + + [Theory] + [InlineData("5")] + [InlineData("\"high\"")] + [InlineData("1.5")] + public void Inspect_MalformedQos_Warns(string qosToken) + { + var warnings = MqttTagDefinitionFactory.Inspect( + $$"""{"topic":"a/b","payloadFormat":"Raw","qos":{{qosToken}}}"""); + warnings.ShouldHaveSingleItem().ShouldContain("qos", Case.Sensitive); + } + + /// Every warning source fires together — the pass reports all of them, not just the first. + [Fact] + public void Inspect_MultipleProblems_ReportsAll() + { + var warnings = MqttTagDefinitionFactory.Inspect( + """{"topic":"a/+/c","payloadFormat":"Jason","dataType":"Double","qos":9}"""); + warnings.Count.ShouldBe(4); + } + + [Fact] + public void Inspect_UnparseableBlob_Warns() + => MqttTagDefinitionFactory.Inspect("{ not json at all").ShouldNotBeEmpty(); + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("factory/oven/temp")] + [InlineData("[1,2,3]")] // no leading '{' ⇒ not a TagConfig blob at all (mirrors Modbus) + public void Inspect_NotATagConfigBlob_ReturnsEmpty(string? reference) + => MqttTagDefinitionFactory.Inspect(reference!).ShouldBeEmpty(); +}