Merge branch 'fix/archreview-u2-deferred-sink-forwarding-guard'
This commit is contained in:
Joseph Doherty
+155
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// Arch-review <b>03/U2</b> — a mechanical, reflection-driven guard that <b>every</b> method of
|
||||
/// <b>every</b> forwarding interface the deferred wrappers implement actually reaches the inner
|
||||
/// sink/publisher. The per-member hand-written tests in <c>DeferredAddressSpaceSinkTests</c> cover
|
||||
/// the members that exist today; this test closes the class of bug where a <i>newly-added</i> sink
|
||||
/// capability is implemented on <c>DeferredAddressSpaceSink</c> as a silent no-op (never forwarded),
|
||||
/// so the optimization ships <b>inert on every driver-role host</b> — the F10b / PR#423 failure mode,
|
||||
/// because actors inject the deferred <i>wrapper</i>, not the inner <c>SdkAddressSpaceSink</c>.
|
||||
///
|
||||
/// A <see cref="DispatchProxy"/> recording inner auto-implements any future interface member with
|
||||
/// zero maintenance, so adding a method to <see cref="IOpcUaAddressSpaceSink"/> or
|
||||
/// <see cref="ISurgicalAddressSpaceSink"/> is covered automatically — if the wrapper forgets to
|
||||
/// forward it, <see cref="Every_forwarding_interface_method_reaches_the_inner_sink"/> fails. Adding a
|
||||
/// brand-new capability <i>interface</i> to the wrapper is caught by
|
||||
/// <see cref="DeferredAddressSpaceSink_implements_exactly_the_known_forwarding_interfaces"/>.
|
||||
/// </summary>
|
||||
public class DeferredSinkForwardingReflectionTests
|
||||
{
|
||||
/// <summary>The interfaces whose members <see cref="DeferredAddressSpaceSink"/> is contractually
|
||||
/// required to forward to its inner sink. Kept in sync with reality by
|
||||
/// <see cref="DeferredAddressSpaceSink_implements_exactly_the_known_forwarding_interfaces"/>.</summary>
|
||||
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<IRecordingSink, RecordingProxy>();
|
||||
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<IServiceLevelPublisher, RecordingProxy>();
|
||||
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.");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Combined interface so a single <see cref="DispatchProxy"/> recorder implements every
|
||||
/// forwarding member of <see cref="IOpcUaAddressSpaceSink"/> + <see cref="ISurgicalAddressSpaceSink"/>
|
||||
/// (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.</summary>
|
||||
public interface IRecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink;
|
||||
|
||||
/// <summary>Records the name of every interface member the wrapper forwards to it. Returns
|
||||
/// <c>true</c> for <see cref="bool"/>-returning members so the wrapper's capability-sniffing
|
||||
/// <c>_inner is ISurgicalAddressSpaceSink surgical && surgical.X(...)</c> short-circuit
|
||||
/// still invokes (and thus records) the member.</summary>
|
||||
public class RecordingProxy : DispatchProxy
|
||||
{
|
||||
/// <summary>Gets the names of the interface members invoked on this proxy.</summary>
|
||||
public List<string> Invoked { get; } = new();
|
||||
|
||||
/// <inheritdoc />
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>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.</summary>
|
||||
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();
|
||||
}
|
||||
Reference in New Issue
Block a user