Joseph Doherty
803f4e6efe
`Driver.Sql.Browser` referenced the runtime `Driver.Sql` project. The AdminUI references that browser, so the whole SQL driver runtime — and `ZB.MOM.WW.OtOpcUa.Core` through it — sat in the AdminUI's build/deploy graph. This is the same layering violation #512 removed from the MQTT browser, in a second driver, and it is why #512's expected removal of `Core.dll` from the AdminUI's output did not happen. MEASURED, both directions, on a clean AdminUI build: before: ZB.MOM.WW.OtOpcUa.Core.dll + ZB.MOM.WW.OtOpcUa.Driver.Sql.dll present after: both absent The old csproj comment's premise was sound and survives — the dialect's catalog SQL *is* the browse engine, so it must be shared rather than duplicated. What was wrong was the conclusion that sharing it required referencing the runtime driver. `ISqlDialect`/`SqlServerDialect` depend on nothing in `Driver.Sql`, so they move to a `.Dialect` leaf that both consume. This is the `.Transport` shape, not the cheap one: `ISqlDialect` exposes `DbProviderFactory` and `SqlServerDialect` returns `SqlClientFactory.Instance`, so `.Contracts` (deliberately provider-free) cannot hold them. Assembly name is `.Dialect`; the NAMESPACE stays `ZB.MOM.WW.OtOpcUa.Driver.Sql`, matching the `Driver.Mqtt.Transport` precedent — the extraction cost zero `using` changes. Two dependencies the cut edge surfaced as the implicit ones they were: - The AdminUI itself read `SqlDriver.DriverTypeName` in three places — the tag editor map, the tag config validator, and the Sql address picker. All three now read `DriverTypeNames.Sql`, which is exactly what that constant resolves to and exactly the repoint `SqlDriverBrowser`'s own comment had deferred until a factory was registered against the shared constant. One exists now. - `SqlDriverFormContractTests` asserts the AdminUI form's emitted JSON against the runtime factory that reads it, so it genuinely needs the runtime driver. `AdminUI.Tests` now references it explicitly. A test project referencing a runtime driver is fine — tests are not in the AdminUI's deploy graph. New guard: `DriverBrowserLayeringTests` (Commons.Tests) scans the csproj graph of every `*.Browser` project in the tree, transitively. It reads csproj files rather than loaded assemblies because the cost is a build-graph cost — the compiler emits no assembly reference for an unused `ProjectReference`, so `MqttBrowserLayeringTests`' assembly-reference approach cannot see that case. The two are complementary and each now points at the other. Falsified three ways, each red then green again: the direct reference re-added to the browser; the same reference one hop out via `.Dialect` (proving the walk is really transitive); and both controls — browsers-discovered and references-parsed — which exist so an empty discovery cannot pass the absence assertion vacuously. #526's claim that the reference "carries no comment at all" was wrong; it carried a detailed one from the scaffold commit. Corrected on the issue and in the design doc's project table, which had recorded the deviation as deliberate. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
94 lines
4.6 KiB
C#
94 lines
4.6 KiB
C#
using System.Reflection;
|
|
using Shouldly;
|
|
using Xunit;
|
|
|
|
namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests;
|
|
|
|
/// <summary>
|
|
/// Pins the MQTT project layering that Gitea #512 restored: <c>.Browser</c> depends on the shared
|
|
/// <c>.Contracts</c> + <c>.Transport</c> leaves, never on the runtime <c>.Driver</c>.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// <para>
|
|
/// <b>Why this is worth a test.</b> <c>.Browser</c> used to reference the runtime driver, and the
|
|
/// AdminUI references <c>.Browser</c> — so the AdminUI's build/deploy graph transitively contained
|
|
/// the whole MQTT driver runtime plus Core, Polly.Core and Serilog. Nothing failed; the cost was
|
|
/// invisible. A reference re-added out of convenience would be invisible in exactly the same way,
|
|
/// which is precisely what a guard is for.
|
|
/// </para>
|
|
/// <para>
|
|
/// <b>What it can and cannot catch.</b> It asserts on emitted assembly references, so it fires when
|
|
/// the browser actually <em>uses</em> a driver-runtime type — the case that matters. A
|
|
/// <c>ProjectReference</c> re-added but never used would not trip it (the compiler emits no
|
|
/// reference), so the csproj comment remains the primary record of the decision.
|
|
/// </para>
|
|
/// <para>
|
|
/// <b>That gap is now covered</b> by <c>DriverBrowserLayeringTests</c> (Commons.Tests), which
|
|
/// scans the csproj graph of every <c>*.Browser</c> project in the tree — added for Gitea #526,
|
|
/// after the identical violation was found sitting in <c>Driver.Sql.Browser</c> with no guard
|
|
/// watching it, because this file was written for the one driver that had an issue filed. What
|
|
/// stays here is the MQTT-specific half: the <c>.Transport</c> leaf shape and the
|
|
/// MQTTnet-free-ness of <c>.Contracts</c>.
|
|
/// </para>
|
|
/// </remarks>
|
|
[Trait("Category", "Unit")]
|
|
public sealed class MqttBrowserLayeringTests
|
|
{
|
|
private const string BrowserAssembly = "ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser";
|
|
private const string DriverRuntimeAssembly = "ZB.MOM.WW.OtOpcUa.Driver.Mqtt";
|
|
private const string ContractsAssembly = "ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts";
|
|
private const string TransportAssembly = "ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport";
|
|
|
|
private static string[] BrowserReferences()
|
|
{
|
|
var browser = Assembly.Load(BrowserAssembly);
|
|
return [.. browser.GetReferencedAssemblies().Select(a => a.Name!)];
|
|
}
|
|
|
|
[Fact]
|
|
public void Browser_does_not_reference_the_runtime_driver()
|
|
=> BrowserReferences().ShouldNotContain(
|
|
DriverRuntimeAssembly,
|
|
"the MQTT browser is back to referencing the runtime driver — every other bespoke browser " +
|
|
"in this repo (OpcUaClient, Galaxy) depends on its .Contracts leaf only, and this reference " +
|
|
"reaches the AdminUI. If shared connection policy is needed, put it in .Transport; if shared " +
|
|
"protocol/codec is needed, put it in .Contracts. Do NOT copy the TLS/CA-pin logic.");
|
|
|
|
[Fact]
|
|
public void Browser_reaches_the_shared_leaves_instead()
|
|
{
|
|
// The positive control. Asserting only the absence above would also pass if the browser stopped
|
|
// referencing anything MQTT at all — e.g. if this test loaded the wrong assembly, or the type
|
|
// names drifted — so pin what it SHOULD depend on in the same breath.
|
|
var references = BrowserReferences();
|
|
|
|
references.ShouldContain(ContractsAssembly);
|
|
references.ShouldContain(TransportAssembly);
|
|
}
|
|
|
|
[Fact]
|
|
public void Transport_stays_a_leaf_that_does_not_reach_back_into_the_runtime_driver()
|
|
{
|
|
// .Transport holds connection POLICY only. If it ever referenced the runtime driver, routing the
|
|
// browser through it would restore the coupling this whole change removed, one hop further out.
|
|
var references = Assembly.Load(TransportAssembly)
|
|
.GetReferencedAssemblies()
|
|
.Select(a => a.Name!)
|
|
.ToArray();
|
|
|
|
references.ShouldNotContain(DriverRuntimeAssembly);
|
|
references.ShouldNotContain(BrowserAssembly);
|
|
}
|
|
|
|
[Fact]
|
|
public void Contracts_stays_transport_free()
|
|
=> Assembly.Load(ContractsAssembly)
|
|
.GetReferencedAssemblies()
|
|
.Select(a => a.Name!)
|
|
.ShouldNotContain(
|
|
"MQTTnet",
|
|
"Driver.Mqtt.Contracts is deliberately transport-free — it carries the Sparkplug wire " +
|
|
"schema and codec, and is referenced by ControlPlane, which has no business with a " +
|
|
"broker client. MQTTnet-typed connection policy belongs in .Transport.");
|
|
}
|