From a65c2ced19be86fd8b2e25211d43917b4fb6daaf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 8 Jul 2026 17:37:46 -0400 Subject: [PATCH] test(commons): reflection-exhaustive deferred-sink forwarding guard (arch-review 03/U2) Add a mechanical, reflection-driven guard that every method of every forwarding interface DeferredAddressSpaceSink implements (IOpcUaAddressSpaceSink + ISurgicalAddressSpaceSink) actually reaches the inner sink, plus the sibling DeferredServiceLevelPublisher. A DispatchProxy recording inner auto-implements future interface members with zero maintenance, so a newly-added sink capability that the wrapper forgets to forward now fails a test instead of shipping inert on every driver-role host (the F10b / PR#423 failure mode). A guard-of-the-guard asserts the wrapper implements exactly the known forwarding-interface set, so a brand-new capability *interface* trips the test too. This is the prerequisite gate for 03/P1 (surgical remove methods) and 02/P (batched WriteValues): both add sink capability members that must not ship inert. Verified: 3 new tests green (Commons.Tests 57/57); negative control (stubbing UpdateFolderDisplayName to a non-forwarding no-op) fails with the exact member-named diagnostic, proving the guard bites. --- .../DeferredSinkForwardingReflectionTests.cs | 155 ++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs new file mode 100644 index 00000000..7903ac34 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs @@ -0,0 +1,155 @@ +using System.Reflection; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; + +namespace ZB.MOM.WW.OtOpcUa.Commons.Tests.OpcUa; + +/// +/// Arch-review 03/U2 — a mechanical, reflection-driven guard that every method of +/// every forwarding interface the deferred wrappers implement actually reaches the inner +/// sink/publisher. The per-member hand-written tests in DeferredAddressSpaceSinkTests cover +/// the members that exist today; this test closes the class of bug where a newly-added sink +/// capability is implemented on DeferredAddressSpaceSink as a silent no-op (never forwarded), +/// so the optimization ships inert on every driver-role host — the F10b / PR#423 failure mode, +/// because actors inject the deferred wrapper, not the inner SdkAddressSpaceSink. +/// +/// A recording inner auto-implements any future interface member with +/// zero maintenance, so adding a method to or +/// is covered automatically — if the wrapper forgets to +/// forward it, fails. Adding a +/// brand-new capability interface to the wrapper is caught by +/// . +/// +public class DeferredSinkForwardingReflectionTests +{ + /// The interfaces whose members is contractually + /// required to forward to its inner sink. Kept in sync with reality by + /// . + private static readonly Type[] ForwardingInterfaces = + { + typeof(IOpcUaAddressSpaceSink), + typeof(ISurgicalAddressSpaceSink), + }; + + [Fact] + public void DeferredAddressSpaceSink_implements_exactly_the_known_forwarding_interfaces() + { + // Guard-of-the-guard: if a new capability interface is added to (or removed from) + // DeferredAddressSpaceSink, this trips — forcing whoever added it to (1) list it in + // ForwardingInterfaces and (2) extend IRecordingSink so the exhaustive forwarding test + // below actually covers its members. Without this, a new interface could ship with an + // un-forwarded member and no test would notice (the F10b / PR#423 trap class). + var actual = typeof(DeferredAddressSpaceSink).GetInterfaces() + .Where(i => i.Namespace == typeof(IOpcUaAddressSpaceSink).Namespace) + .ToHashSet(); + + actual.ShouldBe( + ForwardingInterfaces.ToHashSet(), + ignoreOrder: true, + customMessage: + "A forwarding interface was added to / removed from DeferredAddressSpaceSink without updating " + + "this exhaustive forwarding guard. Update ForwardingInterfaces + IRecordingSink, then re-run."); + } + + [Fact] + public void Every_forwarding_interface_method_reaches_the_inner_sink() + { + foreach (var method in ForwardingInterfaces.SelectMany(i => i.GetMethods())) + { + // Fresh wrapper + recorder per member so Invoked reflects exactly this call. + var proxy = DispatchProxy.Create(); + var recorder = (RecordingProxy)(object)proxy; + + var deferred = new DeferredAddressSpaceSink(); + deferred.SetSink(proxy); + + var args = method.GetParameters().Select(p => DummyArg(p.ParameterType)).ToArray(); + method.Invoke(deferred, args); + + recorder.Invoked.ShouldContain( + method.Name, + $"DeferredAddressSpaceSink.{method.Name} did NOT forward to its inner sink — it ships inert " + + "on every driver-role host (F10b / PR#423 class). Forward it in DeferredAddressSpaceSink.cs."); + } + } + + [Fact] + public void Every_IServiceLevelPublisher_method_reaches_the_inner_publisher() + { + // DeferredServiceLevelPublisher is the sibling late-binding adapter with the same trap; + // the plan (03/U2) calls for the same treatment. + foreach (var method in typeof(IServiceLevelPublisher).GetMethods()) + { + var proxy = DispatchProxy.Create(); + var recorder = (RecordingProxy)(object)proxy; + + var deferred = new DeferredServiceLevelPublisher(); + deferred.SetInner(proxy); + + var args = method.GetParameters().Select(p => DummyArg(p.ParameterType)).ToArray(); + method.Invoke(deferred, args); + + recorder.Invoked.ShouldContain( + method.Name, + $"DeferredServiceLevelPublisher.{method.Name} did NOT forward to its inner publisher."); + } + } + + /// Combined interface so a single recorder implements every + /// forwarding member of + + /// (the wrapper requires an inner that is BOTH for surgical forwarding to engage). New members on + /// either base interface are implemented by the proxy automatically. + public interface IRecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink; + + /// Records the name of every interface member the wrapper forwards to it. Returns + /// true for -returning members so the wrapper's capability-sniffing + /// _inner is ISurgicalAddressSpaceSink surgical && surgical.X(...) short-circuit + /// still invokes (and thus records) the member. + public class RecordingProxy : DispatchProxy + { + /// Gets the names of the interface members invoked on this proxy. + public List Invoked { get; } = new(); + + /// + protected override object? Invoke(MethodInfo? targetMethod, object?[]? args) + { + Invoked.Add(targetMethod!.Name); + var rt = targetMethod.ReturnType; + if (rt == typeof(void)) return null; + if (rt == typeof(bool)) return true; + return rt.IsValueType ? Activator.CreateInstance(rt) : null; + } + } + + /// Builds a non-null dummy argument for any parameter type in the sink interfaces so the + /// reflective invoke can't throw on a null value type. Fidelity of the forwarded args is asserted + /// by the hand-written per-member tests; here we only need the call to arrive. + private static object? DummyArg(Type t) + { + if (t == typeof(string)) return "n"; + if (t == typeof(object)) return 0; + + var underlying = Nullable.GetUnderlyingType(t); + if (underlying != null) return DummyArg(underlying); + + if (t.IsEnum) return Enum.GetValues(t).GetValue(0); + if (t == typeof(DateTime)) return DateTime.UtcNow; + + if (t.IsValueType) + { + if (t.IsPrimitive || t == typeof(decimal)) return Activator.CreateInstance(t); + var vctor = GreediestCtor(t); + return vctor is null + ? Activator.CreateInstance(t) + : vctor.Invoke(vctor.GetParameters().Select(p => DummyArg(p.ParameterType)).ToArray()); + } + + // Reference type with a greedy constructor (e.g. the AlarmConditionSnapshot record). + var ctor = GreediestCtor(t); + return ctor?.Invoke(ctor.GetParameters().Select(p => DummyArg(p.ParameterType)).ToArray()); + } + + private static ConstructorInfo? GreediestCtor(Type t) => + t.GetConstructors().OrderByDescending(c => c.GetParameters().Length).FirstOrDefault(); +}