3cc8069150
SqlDriverFactoryExtensions turns a deployed DriverConfig blob into a live SqlDriver, and SqlConnectionStringResolver resolves the authored connectionStringRef NAME from Sql__ConnectionStrings__<ref> so credentials never ride in a config blob (design §8.2). Three behaviours worth naming: - String-enum guard. JsonOptions carries JsonStringEnumConverter + camelCase, so `provider` parses whether authored as a name or an ordinal and always round-trips as the NAME — the systemic AdminUI defect where a page serialised an enum numerically against a string-typed DTO. - Config validation lands here because nothing below does it. operationTimeout must be STRICTLY greater than commandTimeout (design §8.3); the reader and the driver stay deliberately usable with the pair inverted so the frozen-database tests can prove the client-side bound fires. Non-positive timeouts / poll interval and maxConcurrentGroups < 1 are rejected too. - Credential hygiene. The resolved connection string reaches the provider and nothing else: messages name the ref, the environment variable, or SqlDriver.Endpoint's credential-free server/database rendering. Both a log-leak and an exception-leak test cover it. DTO changes: - Adds RawTags (List<RawTagEntry>), following the Modbus precedent — the deploy artifact delivers a driver's tags this way and the DTO had no way to receive them, so an authored Sql tag could never reach the driver. - Deletes SqlProbeDto + the `probe` key. It had no consumer: SqlDriver was specified without a background probe loop, and deliberately so — its IHostConnectivityProbe state is a by-product of the Initialize liveness check and of every poll, i.e. a statement about traffic that actually happened rather than a synthetic ping. On-demand connectivity is Task 10's SqlDriverProbe. Shipping a config key that silently does nothing is worse than not shipping it; UnmappedMemberHandling.Skip means a blob that still carries `probe` keeps parsing. allowWrites stays inert (SqlDriver implements no write capability) but an authored `true` now WARNS rather than passing silently — the flag cannot do harm, an operator who believes writes are enabled can. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
453 lines
19 KiB
C#
453 lines
19 KiB
C#
using System.Text.Json;
|
|
using Microsoft.Extensions.Logging;
|
|
using Shouldly;
|
|
using Xunit;
|
|
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
|
using ZB.MOM.WW.OtOpcUa.Core.Hosting;
|
|
using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
|
|
|
|
namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests;
|
|
|
|
/// <summary>
|
|
/// Proves the composition layer: <see cref="SqlDriverFactoryExtensions"/> (config blob → live driver) and
|
|
/// <see cref="SqlConnectionStringResolver"/> (a <c>connectionStringRef</c> name → the secret it stands for).
|
|
/// <para>Three things here are behaviour rather than plumbing, and each has its own test below.
|
|
/// <b>(1) The string-enum guard</b> — the driver-page enum-serialization defect that bit the AdminUI shipped
|
|
/// configs whose enum fields were written as ORDINALS while the consuming DTO was string-typed, faulting the
|
|
/// driver at deploy time. The factory's options therefore both accept a numeric <c>provider</c> and write the
|
|
/// NAME. <b>(2) Config validation</b> — <c>operationTimeout > commandTimeout</c> is deliberately
|
|
/// unenforced in the reader and the shell, so the factory is the only place it can be caught before a
|
|
/// deployment silently masks its own server-side backstop. <b>(3) Credential hygiene</b> — the resolved
|
|
/// connection string is a secret; it must not reach a log line or an exception message, and the tests below
|
|
/// assert that rather than trusting the reviewer.</para>
|
|
/// </summary>
|
|
public sealed class SqlDriverFactoryTests
|
|
{
|
|
// ---- the plan's headline legs ----
|
|
|
|
[Fact]
|
|
public void CreateInstance_missingConnectionStringRef_throwsActionable()
|
|
=> Should.Throw<InvalidOperationException>(() =>
|
|
SqlDriverFactoryExtensions.CreateInstance("d1", """{"provider":"SqlServer"}""", null))
|
|
.Message.ShouldContain("connectionStringRef");
|
|
|
|
[Fact]
|
|
public void ConnectionStringRef_resolvesFromEnv()
|
|
{
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=x;Database=y;");
|
|
|
|
SqlConnectionStringResolver.Resolve(name).ShouldBe("Server=x;Database=y;");
|
|
}
|
|
|
|
[Fact]
|
|
public void Provider_serializesAsNameNotNumber()
|
|
{
|
|
var json = JsonSerializer.Serialize(
|
|
new SqlDriverConfigDto { Provider = SqlProvider.SqlServer },
|
|
SqlDriverFactoryExtensions.JsonOptionsForTest);
|
|
|
|
json.ShouldContain("\"SqlServer\"");
|
|
json.ShouldNotContain("\"provider\":0");
|
|
}
|
|
|
|
// ---- the other half of the enum guard: a NUMERIC provider must still parse ----
|
|
|
|
[Fact]
|
|
public void CreateInstance_providerWrittenAsANumber_stillParses()
|
|
{
|
|
// The defect's real shape: an authoring surface serialises the enum as an ordinal. Rejecting that
|
|
// would fault the driver at deploy time, which is exactly what the guard exists to prevent — so the
|
|
// options accept both spellings on the way IN, and only ever emit the name on the way OUT.
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
|
|
var driver = SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1", $$"""{"provider":0,"connectionStringRef":"{{name}}"}""", null);
|
|
|
|
driver.DriverType.ShouldBe(SqlDriver.DriverTypeName);
|
|
}
|
|
|
|
[Fact]
|
|
public void CreateInstance_unknownJsonMembers_areIgnoredRatherThanFatal()
|
|
{
|
|
// A config blob authored against a newer (or older) schema must not brick the driver.
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
|
|
var driver = SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1", $$"""{"connectionStringRef":"{{name}}","somethingNobodyHasShippedYet":true}""", null);
|
|
|
|
driver.DriverInstanceId.ShouldBe("d1");
|
|
}
|
|
|
|
// ---- connection-string resolution ----
|
|
|
|
[Fact]
|
|
public void Resolve_whenTheEnvironmentVariableIsAbsent_throwsNamingTheVariable()
|
|
{
|
|
var name = NewRef();
|
|
|
|
var ex = Should.Throw<InvalidOperationException>(() => SqlConnectionStringResolver.Resolve(name));
|
|
|
|
// The operator's next action is "set this variable", so the message must name it exactly.
|
|
ex.Message.ShouldContain(SqlConnectionStringResolver.EnvironmentVariableFor(name));
|
|
}
|
|
|
|
[Fact]
|
|
public void Resolve_whenTheEnvironmentVariableIsBlank_isTreatedAsAbsent()
|
|
{
|
|
// An empty value is a half-provisioned deployment, not a connection string — failing here beats
|
|
// handing "" to the provider and reporting an unintelligible ADO.NET error.
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), " ");
|
|
|
|
Should.Throw<InvalidOperationException>(() => SqlConnectionStringResolver.Resolve(name));
|
|
}
|
|
|
|
[Fact]
|
|
public void CreateInstance_whenTheRefResolvesToNothing_namesTheEnvironmentVariable_notTheRef()
|
|
{
|
|
var name = NewRef();
|
|
|
|
var ex = Should.Throw<InvalidOperationException>(() => SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1", $$"""{"connectionStringRef":"{{name}}"}""", null));
|
|
|
|
ex.Message.ShouldContain(SqlConnectionStringResolver.EnvironmentVariableFor(name));
|
|
}
|
|
|
|
// ---- provider gate ----
|
|
|
|
[Fact]
|
|
public void CreateInstance_aProviderThisBuildCannotConstruct_throwsSayingSo()
|
|
{
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Host=h;Database=db");
|
|
|
|
var ex = Should.Throw<InvalidOperationException>(() => SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1", $$"""{"provider":"Postgres","connectionStringRef":"{{name}}"}""", null));
|
|
|
|
ex.Message.ShouldContain("Postgres");
|
|
ex.Message.ShouldContain("not available in this build");
|
|
}
|
|
|
|
// ---- config validation (the reader and the shell deliberately do NOT do this) ----
|
|
|
|
[Fact]
|
|
public void CreateInstance_operationTimeoutNotGreaterThanCommandTimeout_isRejected()
|
|
{
|
|
// Inverted, the client-side wall-clock abort always fires first and the server-side CommandTimeout
|
|
// backstop can never be reached — the deployment silently loses one of its two independent bounds.
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
|
|
var ex = Should.Throw<InvalidOperationException>(() => SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1",
|
|
$$"""
|
|
{"connectionStringRef":"{{name}}","commandTimeout":"00:00:30","operationTimeout":"00:00:15"}
|
|
""",
|
|
null));
|
|
|
|
ex.Message.ShouldContain("operationTimeout");
|
|
ex.Message.ShouldContain("commandTimeout");
|
|
}
|
|
|
|
[Fact]
|
|
public void CreateInstance_equalTimeouts_areAlsoRejected()
|
|
{
|
|
// "Strictly greater" (design §8.3): equal leaves the two bounds racing, which is the same defect
|
|
// with a coin toss on top.
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
|
|
Should.Throw<InvalidOperationException>(() => SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1",
|
|
$$"""
|
|
{"connectionStringRef":"{{name}}","commandTimeout":"00:00:10","operationTimeout":"00:00:10"}
|
|
""",
|
|
null));
|
|
}
|
|
|
|
[Theory]
|
|
[InlineData("\"commandTimeout\":\"00:00:00\"")]
|
|
[InlineData("\"operationTimeout\":\"-00:00:05\"")]
|
|
[InlineData("\"defaultPollInterval\":\"00:00:00\"")]
|
|
[InlineData("\"maxConcurrentGroups\":0")]
|
|
public void CreateInstance_nonPositiveKnobs_areRejected(string knobJson)
|
|
{
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
|
|
Should.Throw<InvalidOperationException>(() => SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1", $$"""{"connectionStringRef":"{{name}}",{{knobJson}}}""", null));
|
|
}
|
|
|
|
[Fact]
|
|
public void CreateInstance_theDocumentedDefaults_passValidation()
|
|
{
|
|
// The falsifiability control for the four rejection legs above: an all-defaults config must build.
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
|
|
var options = SqlDriverFactoryExtensions.BuildOptions(
|
|
new SqlDriverConfigDto { ConnectionStringRef = name }, "d1");
|
|
|
|
options.CommandTimeout.ShouldBe(TimeSpan.FromSeconds(10));
|
|
options.OperationTimeout.ShouldBe(TimeSpan.FromSeconds(15));
|
|
options.DefaultPollInterval.ShouldBe(TimeSpan.FromSeconds(5));
|
|
options.MaxConcurrentGroups.ShouldBe(4);
|
|
options.NullIsBad.ShouldBeFalse();
|
|
options.OperationTimeout.ShouldBeGreaterThan(options.CommandTimeout);
|
|
}
|
|
|
|
// ---- rawTags: how the deploy artifact actually delivers a driver's tags ----
|
|
|
|
[Fact]
|
|
public void BuildOptions_carriesTheArtifactsRawTagsOntoTheOptions()
|
|
{
|
|
var dto = JsonSerializer.Deserialize<SqlDriverConfigDto>(
|
|
"""
|
|
{
|
|
"connectionStringRef":"anything",
|
|
"rawTags":[
|
|
{"rawPath":"Plant/Sql/db1/Speed","tagConfig":"{\"driver\":\"Sql\"}","writeIdempotent":false},
|
|
{"rawPath":"Plant/Sql/db1/Temp","tagConfig":"{\"driver\":\"Sql\"}","writeIdempotent":false}
|
|
]
|
|
}
|
|
""",
|
|
SqlDriverFactoryExtensions.JsonOptionsForTest)!;
|
|
|
|
var options = SqlDriverFactoryExtensions.BuildOptions(dto, "d1");
|
|
|
|
options.RawTags.Count.ShouldBe(2);
|
|
options.RawTags[0].RawPath.ShouldBe("Plant/Sql/db1/Speed");
|
|
options.RawTags[1].RawPath.ShouldBe("Plant/Sql/db1/Temp");
|
|
}
|
|
|
|
[Fact]
|
|
public void BuildOptions_withNoRawTags_yieldsAnEmptyList_notNull()
|
|
{
|
|
var options = SqlDriverFactoryExtensions.BuildOptions(
|
|
new SqlDriverConfigDto { ConnectionStringRef = "anything" }, "d1");
|
|
|
|
options.RawTags.ShouldBeEmpty();
|
|
}
|
|
|
|
// ---- v1 is read-only, structurally ----
|
|
|
|
[Fact]
|
|
public void CreateInstance_anAuthoredAllowWrites_warnsLoudly_andTheDriverStaysReadOnly()
|
|
{
|
|
// allowWrites is inert by construction (IWritable is not implemented), so honouring it is impossible
|
|
// and rejecting it would brick a deployment over a flag that cannot do harm. The remaining failure
|
|
// mode is an operator who believes writes are enabled — which only a loud warning closes.
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
var loggerFactory = new RecordingLoggerFactory();
|
|
|
|
var driver = SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1", $$"""{"connectionStringRef":"{{name}}","allowWrites":true}""", loggerFactory);
|
|
|
|
driver.ShouldNotBeAssignableTo<IWritable>();
|
|
var warning = loggerFactory.Entries
|
|
.Where(e => e.Level >= LogLevel.Warning)
|
|
.ShouldHaveSingleItem();
|
|
warning.Message.ShouldContain("allowWrites");
|
|
warning.Message.ShouldContain("read-only");
|
|
}
|
|
|
|
[Fact]
|
|
public void CreateInstance_withoutAllowWrites_logsNoWarning()
|
|
{
|
|
// The falsifiability control for the warning above — a warning that always fires proves nothing.
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
var loggerFactory = new RecordingLoggerFactory();
|
|
|
|
SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1", $$"""{"connectionStringRef":"{{name}}","allowWrites":false}""", loggerFactory);
|
|
|
|
loggerFactory.Entries.Where(e => e.Level >= LogLevel.Warning).ShouldBeEmpty();
|
|
// ...and the factory DID log — so "no warning" is a statement about severity, not about silence.
|
|
loggerFactory.Entries.ShouldNotBeEmpty();
|
|
}
|
|
|
|
// ---- credential hygiene ----
|
|
|
|
[Fact]
|
|
public void CreateInstance_neverPutsTheResolvedConnectionStringIntoALogOrTheEndpoint()
|
|
{
|
|
const string secret = "SuperSecret-PleaseDoNotLogMe";
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name),
|
|
$"Server=srv;Database=db;User ID=sa;Password={secret}");
|
|
var loggerFactory = new RecordingLoggerFactory();
|
|
|
|
var driver = (SqlDriver)SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1", $$"""{"connectionStringRef":"{{name}}","allowWrites":true}""", loggerFactory);
|
|
|
|
// The credential-free rendering IS emitted — the factory logs where the driver points, which is what
|
|
// makes "no connection string" a real constraint rather than "log nothing and call it safe".
|
|
driver.Endpoint.ShouldBe("srv/db");
|
|
foreach (var entry in loggerFactory.Entries) entry.Message.ShouldNotContain(secret);
|
|
loggerFactory.Entries.ShouldNotBeEmpty(); // ...and there WAS something to leak into.
|
|
}
|
|
|
|
[Fact]
|
|
public void CreateInstance_whenValidationFails_theExceptionCarriesNoConnectionString()
|
|
{
|
|
const string secret = "SuperSecret-PleaseDoNotThrowMe";
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name),
|
|
$"Server=srv;Database=db;Password={secret}");
|
|
|
|
// Validation deliberately runs AFTER resolution here, so the throwing path is one that HAS the
|
|
// secret in hand — the case where a careless $"...{connectionString}" would actually leak.
|
|
var ex = Should.Throw<InvalidOperationException>(() => SqlDriverFactoryExtensions.CreateInstance(
|
|
"d1",
|
|
$$"""
|
|
{"connectionStringRef":"{{name}}","commandTimeout":"00:00:30","operationTimeout":"00:00:15"}
|
|
""",
|
|
null));
|
|
|
|
ex.ToString().ShouldNotContain(secret);
|
|
}
|
|
|
|
// ---- registry wiring ----
|
|
|
|
[Fact]
|
|
public void Register_registersTheFactoryUnderTheDriversTypeName()
|
|
{
|
|
var name = NewRef();
|
|
using var _ = new EnvironmentVariableScope(
|
|
SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db");
|
|
var registry = new DriverFactoryRegistry();
|
|
|
|
SqlDriverFactoryExtensions.Register(registry);
|
|
|
|
var factory = registry.TryGet(SqlDriverFactoryExtensions.DriverTypeName).ShouldNotBeNull();
|
|
var driver = factory("d1", $$"""{"connectionStringRef":"{{name}}"}""");
|
|
driver.ShouldBeOfType<SqlDriver>();
|
|
driver.DriverType.ShouldBe("Sql");
|
|
}
|
|
|
|
[Fact]
|
|
public void DriverTypeName_matchesTheDriversOwnConstant()
|
|
// Task 11 unifies both onto DriverTypeNames.Sql; until then they must not drift apart.
|
|
=> SqlDriverFactoryExtensions.DriverTypeName.ShouldBe(SqlDriver.DriverTypeName);
|
|
|
|
// ---- argument guards ----
|
|
|
|
[Theory]
|
|
[InlineData("")]
|
|
[InlineData(" ")]
|
|
public void CreateInstance_blankInputs_areRejected(string blank)
|
|
{
|
|
Should.Throw<ArgumentException>(() =>
|
|
SqlDriverFactoryExtensions.CreateInstance(blank, """{"connectionStringRef":"x"}""", null));
|
|
Should.Throw<ArgumentException>(() => SqlDriverFactoryExtensions.CreateInstance("d1", blank, null));
|
|
}
|
|
|
|
[Fact]
|
|
public void CreateInstance_configThatIsNotJson_throwsNamingTheDriverInstance()
|
|
{
|
|
var ex = Should.Throw<InvalidOperationException>(
|
|
() => SqlDriverFactoryExtensions.CreateInstance("d1", "not json at all", null));
|
|
|
|
ex.Message.ShouldContain("d1");
|
|
}
|
|
|
|
/// <summary>A per-test connection-string ref, so no two tests can collide over one process-wide env var.</summary>
|
|
private static string NewRef() => $"OtOpcUaSqlTest{Guid.NewGuid():N}";
|
|
}
|
|
|
|
/// <summary>
|
|
/// Sets one environment variable for the life of the scope and restores its previous value — including
|
|
/// restoring it to <see langword="null"/>. Environment variables are process-global, so a test that sets
|
|
/// one without restoring it leaks into every test that runs after it in the same process.
|
|
/// </summary>
|
|
internal sealed class EnvironmentVariableScope : IDisposable
|
|
{
|
|
private readonly string _name;
|
|
private readonly string? _previous;
|
|
|
|
/// <summary>Sets <paramref name="name"/> to <paramref name="value"/>, remembering what was there.</summary>
|
|
/// <param name="name">The environment variable to set.</param>
|
|
/// <param name="value">The value to set it to.</param>
|
|
public EnvironmentVariableScope(string name, string? value)
|
|
{
|
|
_name = name;
|
|
_previous = Environment.GetEnvironmentVariable(name);
|
|
Environment.SetEnvironmentVariable(name, value);
|
|
}
|
|
|
|
/// <summary>Restores the previous value.</summary>
|
|
public void Dispose() => Environment.SetEnvironmentVariable(_name, _previous);
|
|
}
|
|
|
|
/// <summary>
|
|
/// Captures what the factory logged, so "warned" and "did not leak the connection string" are asserted
|
|
/// rather than assumed.
|
|
/// </summary>
|
|
internal sealed class RecordingLoggerFactory : ILoggerFactory
|
|
{
|
|
private readonly List<LogEntry> _entries = [];
|
|
|
|
/// <summary>The log entries recorded so far, in order.</summary>
|
|
public IReadOnlyList<LogEntry> Entries
|
|
{
|
|
get { lock (_entries) return [.. _entries]; }
|
|
}
|
|
|
|
/// <inheritdoc/>
|
|
public ILogger CreateLogger(string categoryName) => new Recorder(this);
|
|
|
|
/// <inheritdoc/>
|
|
public void AddProvider(ILoggerProvider provider) { }
|
|
|
|
/// <inheritdoc/>
|
|
public void Dispose() { }
|
|
|
|
private void Record(LogLevel level, string message)
|
|
{
|
|
lock (_entries) _entries.Add(new LogEntry(level, message));
|
|
}
|
|
|
|
/// <summary>One captured log entry.</summary>
|
|
/// <param name="Level">The level it was logged at.</param>
|
|
/// <param name="Message">The formatted message.</param>
|
|
internal sealed record LogEntry(LogLevel Level, string Message);
|
|
|
|
private sealed class Recorder(RecordingLoggerFactory owner) : ILogger
|
|
{
|
|
public IDisposable BeginScope<TState>(TState state) where TState : notnull => NullScope.Instance;
|
|
|
|
public bool IsEnabled(LogLevel logLevel) => true;
|
|
|
|
public void Log<TState>(
|
|
LogLevel logLevel, EventId eventId, TState state, Exception? exception,
|
|
Func<TState, Exception?, string> formatter)
|
|
=> owner.Record(logLevel, formatter(state, exception));
|
|
}
|
|
|
|
private sealed class NullScope : IDisposable
|
|
{
|
|
public static readonly NullScope Instance = new();
|
|
|
|
public void Dispose() { }
|
|
}
|
|
}
|