Merge: fix systemic driver-config enum serialization (AdminUI authoring)
v2-ci / build (push) Failing after 38s
v2-ci / unit-tests (tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests) (push) Has been skipped
v2-ci / build (push) Failing after 38s
v2-ci / unit-tests (tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests) (push) Has been skipped
AdminUI driver-instance pages serialized enum config fields as JSON numbers, but the driver factories' DTOs are string-typed + ParseEnum and throw on a number — so AdminUI-authored configs with any enum field faulted the driver on deploy. Add JsonStringEnumConverter to all 9 driver pages + 8 probes (mirroring OpcUaClient). Found + fixed + live-verified by running the never-before-run FB-9 (Modbus-Int64 authoring) and FB-10 (S7/AbCip probe) verifies: an AdminUI-authored S7 driver now persists "cpuType":"S71500" and connects to the sim; the probe E2E is 11/11 green.
This commit is contained in:
@@ -39,6 +39,7 @@ public sealed class AbCipDriverProbe : IDriverProbe
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
@@ -39,6 +39,7 @@ public sealed class AbLegacyDriverProbe : IDriverProbe
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
@@ -44,6 +44,7 @@ public sealed class FocasDriverProbe : IDriverProbe
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
@@ -35,6 +35,7 @@ public sealed class GalaxyDriverProbe : IDriverProbe
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
+1
@@ -24,6 +24,7 @@ public sealed class WonderwareHistorianDriverProbe : IDriverProbe
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
@@ -20,6 +20,7 @@ public sealed class ModbusDriverProbe : IDriverProbe
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
// FC03 Read Holding Registers: function=0x03, addr-hi=0, addr-lo=0, qty-hi=0, qty-lo=1
|
||||
|
||||
@@ -24,6 +24,7 @@ public sealed class S7DriverProbe : IDriverProbe
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
@@ -68,6 +68,7 @@ public sealed class TwinCATDriverProbe : IDriverProbe
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
|
||||
+1
@@ -224,6 +224,7 @@ else
|
||||
PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+1
@@ -190,6 +190,7 @@ else
|
||||
PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+1
@@ -267,6 +267,7 @@ else
|
||||
PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+1
@@ -218,6 +218,7 @@ else
|
||||
PropertyNameCaseInsensitive = true,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+1
@@ -166,6 +166,7 @@ else
|
||||
PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+1
@@ -330,6 +330,7 @@ else
|
||||
PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+1
@@ -279,6 +279,7 @@ else
|
||||
PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+1
@@ -196,6 +196,7 @@ else
|
||||
PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+1
@@ -206,6 +206,7 @@ else
|
||||
PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
|
||||
UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Skip,
|
||||
WriteIndented = false,
|
||||
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
private FormModel _form = new();
|
||||
|
||||
+49
@@ -0,0 +1,49 @@
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Regression guard for the 2026-06-19 enum-serialization bug (the FB-9 Modbus-Int64 authoring
|
||||
/// case). The AdminUI Modbus page now serialises tag enums (<see cref="ModbusRegion"/>,
|
||||
/// <see cref="ModbusDataType"/>, byte-order) as STRINGS. This proves the factory parses that
|
||||
/// AdminUI-shaped Int64-tag blob, and documents that the pre-fix NUMERIC form threw because the
|
||||
/// <c>ModbusTagDto</c> enum fields are <c>string?</c>.
|
||||
/// </summary>
|
||||
public sealed class ModbusDriverConfigEnumSerializationTests
|
||||
{
|
||||
// Mirrors the (now fixed) AdminUI ModbusDriverPage._jsonOpts: camelCase + string enums.
|
||||
private static readonly JsonSerializerOptions _adminPageOpts = new()
|
||||
{
|
||||
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <summary>Verifies the factory parses an AdminUI-authored Modbus config carrying an Int64
|
||||
/// holding-register tag (string enums) without throwing.</summary>
|
||||
[Fact]
|
||||
public void Factory_parses_admin_authored_int64_string_enum_config()
|
||||
{
|
||||
var tag = new ModbusTagDefinition("Int64Tag", ModbusRegion.HoldingRegisters, (ushort)100, ModbusDataType.Int64);
|
||||
var opts = new ModbusDriverOptions { Host = "10.0.0.5", Port = 502, UnitId = 1, Tags = new[] { tag } };
|
||||
var blob = JsonSerializer.Serialize(opts, _adminPageOpts);
|
||||
// The fixed AdminUI page must emit tag enums as strings, not numbers.
|
||||
blob.ShouldContain("\"dataType\":\"Int64\"");
|
||||
|
||||
using var driver = ModbusDriverFactoryExtensions.CreateInstance("mb-test", blob);
|
||||
driver.DriverType.ShouldBe("Modbus");
|
||||
}
|
||||
|
||||
/// <summary>Documents the original bug: the pre-fix AdminUI page emitted numeric tag enums
|
||||
/// (<c>"dataType":5,"region":3</c>) which the string-typed tag DTO cannot bind, so the factory throws.</summary>
|
||||
[Fact]
|
||||
public void Factory_throws_on_the_numeric_enum_form_the_pre_fix_page_emitted()
|
||||
{
|
||||
const string numericBlob =
|
||||
"{\"host\":\"10.0.0.5\",\"port\":502,\"unitId\":1,\"tags\":[" +
|
||||
"{\"name\":\"Int64Tag\",\"region\":3,\"address\":100,\"dataType\":5}]}";
|
||||
Should.Throw<Exception>(() => ModbusDriverFactoryExtensions.CreateInstance("mb-test", numericBlob));
|
||||
}
|
||||
}
|
||||
+48
@@ -0,0 +1,48 @@
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Regression guard for the 2026-06-19 enum-serialization bug. The AdminUI S7 page now
|
||||
/// serialises <see cref="S7CpuType"/> as a STRING (its <c>_jsonOpts</c> gained a
|
||||
/// <see cref="JsonStringEnumConverter"/>). This proves the factory parses that AdminUI-shaped
|
||||
/// blob and round-trips the enum, and documents that the pre-fix NUMERIC form — which the old
|
||||
/// page emitted — threw because <c>S7DriverConfigDto.CpuType</c> is <c>string?</c>.
|
||||
/// </summary>
|
||||
public sealed class S7DriverConfigEnumSerializationTests
|
||||
{
|
||||
// Mirrors the (now fixed) AdminUI S7DriverPage._jsonOpts: camelCase + string enums.
|
||||
private static readonly JsonSerializerOptions _adminPageOpts = new()
|
||||
{
|
||||
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
|
||||
Converters = { new JsonStringEnumConverter() },
|
||||
};
|
||||
|
||||
/// <summary>Verifies the factory parses an AdminUI-authored (string-enum) S7 config and
|
||||
/// preserves the CpuType.</summary>
|
||||
[Fact]
|
||||
public void Factory_parses_admin_authored_string_enum_config()
|
||||
{
|
||||
var opts = new S7DriverOptions { Host = "10.0.0.5", Port = 102, CpuType = S7CpuType.S71500, Rack = 0, Slot = 1 };
|
||||
var blob = JsonSerializer.Serialize(opts, _adminPageOpts);
|
||||
// The fixed AdminUI page must emit the enum as a string, not a number.
|
||||
blob.ShouldContain("\"cpuType\":\"S71500\"");
|
||||
|
||||
var parsed = S7DriverFactoryExtensions.ParseOptions("s7-test", blob);
|
||||
parsed.CpuType.ShouldBe(S7CpuType.S71500);
|
||||
}
|
||||
|
||||
/// <summary>Documents the original bug: the pre-fix AdminUI page emitted a numeric enum
|
||||
/// (<c>"cpuType":40</c>) which the string-typed config DTO cannot bind, so the factory throws.</summary>
|
||||
[Fact]
|
||||
public void Factory_throws_on_the_numeric_enum_form_the_pre_fix_page_emitted()
|
||||
{
|
||||
// 40 == (int)S7CpuType.S71500 — exactly what the pre-fix page (no converter) wrote for S71500.
|
||||
// The throw comes from binding a JSON number to the DTO's string? CpuType, so it fires for any number.
|
||||
const string numericBlob = "{\"host\":\"10.0.0.5\",\"port\":102,\"cpuType\":40,\"rack\":0,\"slot\":1}";
|
||||
Should.Throw<Exception>(() => S7DriverFactoryExtensions.ParseOptions("s7-test", numericBlob));
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,69 @@
|
||||
using System.Reflection;
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Pages.Clusters.Drivers;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Regression guard for the systemic driver-config enum-serialization bug found 2026-06-19.
|
||||
/// Every <c>*DriverPage</c> serialised enum config fields (e.g. S7 <c>CpuType</c>, Modbus
|
||||
/// <c>DataType</c>/<c>Region</c>, AbCip <c>PlcFamily</c>) as JSON <em>numbers</em> because its
|
||||
/// private static <c>_jsonOpts</c> had no <see cref="JsonStringEnumConverter"/>. The driver
|
||||
/// factories, however, deserialise into string-typed DTOs (+ lenient <c>ParseEnum</c>) and
|
||||
/// <em>throw</em> when binding a JSON number to a <c>string?</c> — so an AdminUI-authored
|
||||
/// config that contained any enum field produced a blob the driver could not parse, faulting
|
||||
/// the driver on deploy. The fix makes every page serialise enums as strings (matching the
|
||||
/// factory + the long-correct OpcUaClient template). This test fails if any driver page loses
|
||||
/// its string-enum converter again.
|
||||
/// </summary>
|
||||
public sealed class DriverPageJsonConverterTests
|
||||
{
|
||||
/// <summary>Every concrete <c>*DriverPage</c> in the AdminUI assembly that declares a
|
||||
/// <c>_jsonOpts</c> config serializer.</summary>
|
||||
private static IReadOnlyList<Type> DriverPageTypes { get; } =
|
||||
typeof(S7DriverPage).Assembly.GetTypes()
|
||||
.Where(t => t.Name.EndsWith("DriverPage", StringComparison.Ordinal) && !t.IsAbstract)
|
||||
.Where(t => t.GetField("_jsonOpts", BindingFlags.NonPublic | BindingFlags.Static) is not null)
|
||||
.OrderBy(t => t.Name)
|
||||
.ToList();
|
||||
|
||||
/// <summary>xUnit theory source over the driver-page types discovered by reflection.</summary>
|
||||
public static TheoryData<Type> DriverPagesWithJsonOpts()
|
||||
{
|
||||
var data = new TheoryData<Type>();
|
||||
foreach (var t in DriverPageTypes)
|
||||
data.Add(t);
|
||||
return data;
|
||||
}
|
||||
|
||||
/// <summary>Verifies every driver page's config serializer registers a string-enum converter so
|
||||
/// enum config fields round-trip as strings (and the driver factory can parse the result).</summary>
|
||||
/// <param name="pageType">A driver page component type discovered by reflection.</param>
|
||||
[Theory]
|
||||
[MemberData(nameof(DriverPagesWithJsonOpts))]
|
||||
public void Driver_page_json_options_register_string_enum_converter(Type pageType)
|
||||
{
|
||||
var field = pageType.GetField("_jsonOpts", BindingFlags.NonPublic | BindingFlags.Static);
|
||||
var opts = (JsonSerializerOptions)field!.GetValue(null)!;
|
||||
opts.Converters.OfType<JsonStringEnumConverter>().ShouldNotBeEmpty(
|
||||
$"{pageType.Name}._jsonOpts must register a JsonStringEnumConverter; otherwise AdminUI-authored " +
|
||||
"enum config fields serialise as numbers and the string-typed driver factory throws on parse.");
|
||||
}
|
||||
|
||||
/// <summary>Enforces that EVERY concrete <c>*DriverPage</c> routes config serialization through a
|
||||
/// <c>_jsonOpts</c> field — otherwise a new page that serialised config a different way would slip
|
||||
/// past the converter guard above. Also a floor check so a rename can't silently shrink the set.</summary>
|
||||
[Fact]
|
||||
public void Every_driver_page_uses_a_guarded_jsonOpts_serializer()
|
||||
{
|
||||
var allDriverPages = typeof(S7DriverPage).Assembly.GetTypes()
|
||||
.Where(t => t.Name.EndsWith("DriverPage", StringComparison.Ordinal) && !t.IsAbstract)
|
||||
.ToList();
|
||||
allDriverPages.Count.ShouldBeGreaterThanOrEqualTo(9, "reflection should discover the full driver-page fleet");
|
||||
DriverPageTypes.Count.ShouldBe(allDriverPages.Count,
|
||||
"every *DriverPage must declare a _jsonOpts config serializer so the string-enum converter guard covers it");
|
||||
}
|
||||
}
|
||||
+9
-1
@@ -172,8 +172,16 @@ public sealed class DriverProbeHandshakeE2eTests
|
||||
public async Task AbCip_Green_AgainstSim()
|
||||
{
|
||||
SkipUnless(DockerHost, AbCipPort);
|
||||
// Probe an explicit tag that the ab_server ControlLogix sim actually defines
|
||||
// (`TestDINT:DINT[1]`). The no-tags fallback (`@raw_cpu_type`) is NOT exercised here:
|
||||
// ab_server answers an unknown/unsupported tag with libplctag ErrorBadParam (a REAL
|
||||
// ControlLogix instead returns ErrorNotFound, which the probe classifies as
|
||||
// reachable). Whether the `@raw_cpu_type` system-tag fallback is valid on a real
|
||||
// ControlLogix is a hardware-gated follow-up (AbCipDriverOptions.cs flags it deferred).
|
||||
var result = await new AbCipDriverProbe().ProbeAsync(
|
||||
$"{{\"Devices\":[{{\"HostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}]}}", Timeout, Ct);
|
||||
$"{{\"Devices\":[{{\"HostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}]," +
|
||||
$"\"Tags\":[{{\"DeviceHostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\",\"TagPath\":\"TestDINT\"}}]}}",
|
||||
Timeout, Ct);
|
||||
result.Ok.ShouldBeTrue($"Probe message: {result.Message}");
|
||||
result.Message!.ShouldContain("CIP session OK");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user