diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlDriverConfigDto.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlDriverConfigDto.cs index fac5ee4c..2bca5d24 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlDriverConfigDto.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlDriverConfigDto.cs @@ -1,3 +1,5 @@ +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; /// @@ -41,19 +43,22 @@ public sealed class SqlDriverConfigDto /// When , a NULL cell publishes Bad rather than the default Uncertain. public bool? NullIsBad { get; init; } - /// Master write kill-switch. v1 is read-only — writes stay disabled regardless. + /// + /// Master write kill-switch. v1 is read-only and this field is inert — the driver does not + /// implement IWritable, so no value here can produce a write. The factory warns when it + /// is authored rather than failing the driver, since the flag cannot do harm but + /// an operator who believes writes are enabled can. + /// public bool? AllowWrites { get; init; } - /// Background connectivity-probe settings. Absent ⇒ the factory default. - public SqlProbeDto? Probe { get; init; } -} - -/// Background connectivity-probe settings inside a (design §5.1). -public sealed class SqlProbeDto -{ - /// Whether the driver runs its background connectivity probe. - public bool? Enabled { get; init; } - - /// Interval between probe attempts. - public TimeSpan? Interval { get; init; } + /// + /// The authored raw tags the deploy artifact delivers for this driver instance — RawPath identity plus + /// the driver TagConfig blob, the same shape every other driver receives. The factory copies + /// these onto the driver's options and the driver maps each through + /// at Initialize. + /// A SQL source has no tag-discovery protocol, so this list is the complete set of tags the + /// instance serves — an absent or empty list is a driver with nothing to poll, not a driver that will + /// find its tags later. + /// + public List? RawTags { get; init; } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlConnectionStringResolver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlConnectionStringResolver.cs new file mode 100644 index 00000000..0e76e68d --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlConnectionStringResolver.cs @@ -0,0 +1,62 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; + +/// +/// Resolves a Sql driver config's connectionStringRef — a NAME — into the connection string +/// it stands for (design §8.2). The deployed artifact carries only the name, so database credentials never +/// ride in a config blob, never land in the central config DB, and never appear in a deployment diff. +/// The environment is read directly, on purpose. A driver factory is invoked through +/// DriverFactoryRegistry's (driverInstanceId, driverConfigJson) closure — there is no +/// IConfiguration, no IServiceProvider and no ambient scope at that seam, and widening the +/// registry's signature for one driver would change every driver in the tree. The +/// Sql__ConnectionStrings__<ref> spelling is exactly the key .NET's environment-variable +/// configuration provider would bind to Sql:ConnectionStrings:<ref>, so an operator can set it +/// the same way they set every other secret and a later move onto IConfiguration needs no +/// re-provisioning. +/// Nothing here ever emits the resolved value. Messages name the environment +/// variable, which is not a secret and is the operator's next action; the value itself is returned +/// to exactly one caller and is otherwise unmentioned. +/// +public static class SqlConnectionStringResolver +{ + /// + /// The environment-variable prefix a connectionStringRef is appended to. The double underscore + /// is .NET's configuration hierarchy separator, so this is the environment spelling of + /// Sql:ConnectionStrings:<ref>. + /// + public const string EnvironmentVariablePrefix = "Sql__ConnectionStrings__"; + + /// The environment variable a given connectionStringRef is read from. + /// The authored reference name. + /// The full environment-variable name. + /// is null, empty or whitespace. + public static string EnvironmentVariableFor(string connectionStringRef) + { + ArgumentException.ThrowIfNullOrWhiteSpace(connectionStringRef); + return string.Concat(EnvironmentVariablePrefix, connectionStringRef); + } + + /// + /// Resolves to its connection string. + /// An absent or blank variable is a half-provisioned deployment and throws: handing an + /// empty string to the provider would surface as an unintelligible ADO.NET error at the first poll, + /// far from the missing secret that actually caused it. + /// + /// The authored reference name. + /// The resolved connection string. Treat as a secret — never log it. + /// is null, empty or whitespace. + /// The environment variable is absent or blank. + public static string Resolve(string connectionStringRef) + { + var variable = EnvironmentVariableFor(connectionStringRef); + var value = Environment.GetEnvironmentVariable(variable); + if (string.IsNullOrWhiteSpace(value)) + { + throw new InvalidOperationException( + $"Sql connection string '{connectionStringRef}' is not provisioned: set the environment " + + $"variable '{variable}' on every node that runs this driver. The connection string is " + + $"deliberately not carried in the deployed configuration."); + } + + return value; + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs new file mode 100644 index 00000000..b1377fd5 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs @@ -0,0 +1,255 @@ +using System.Text.Json; +using System.Text.Json.Serialization; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.OtOpcUa.Core.Hosting; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; + +/// +/// Static factory registration helper for — the seam between a deployed +/// DriverConfig blob and a live driver instance. The Server's composition root calls +/// once at startup; the bootstrapper then materialises Sql DriverInstance +/// rows into driver instances. Mirrors ModbusDriverFactoryExtensions. +/// This type owns the config validation the layers below deliberately skip. +/// and stay usable with an inverted +/// operationTimeout/commandTimeout pair on purpose (the frozen-database tests need that +/// shape), so the factory is the only place a deployment can be stopped before it silently loses one of +/// its two independent deadlines. Every knob is checked here, once, at construction. +/// Enum fields are read leniently and written as names. carries a +/// , so a provider authored as an ordinal still parses and a +/// round-trip emits "SqlServer" — the systemic AdminUI defect where a page serialised an enum +/// numerically while the consuming DTO was string-typed, faulting the driver at deploy time. +/// The resolved connection string is a secret and never leaves this method except into the +/// driver. Every log line and every exception message here names the connectionStringRef, the +/// environment variable, or 's credential-free server/database +/// rendering — never the string itself. +/// +public static class SqlDriverFactoryExtensions +{ + /// + /// The driver-type string this factory registers under. + /// Chained off rather than DriverTypeNames.Sql: + /// DriverTypeNamesGuardTests asserts bidirectional parity between the shared constants and the + /// factories actually registered in the process, so the constant may only be added in the change that + /// also wires this factory into the Host (the plan's Task 11, which repoints both at + /// DriverTypeNames.Sql). + /// + public const string DriverTypeName = SqlDriver.DriverTypeName; + + /// + /// How a Sql config blob is read. + /// + /// — accepts an enum written as a name OR as an + /// ordinal, and always writes the name (the enum-serialization guard). + /// — a blob authored against a different + /// schema version must not brick the driver. + /// — the authored spelling + /// (connectionStringRef, rawTags). Reads are case-insensitive anyway; the policy is + /// what makes a write round-trip to the authored shape. + /// + /// + internal static readonly JsonSerializerOptions JsonOptions = new() + { + PropertyNameCaseInsensitive = true, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + ReadCommentHandling = JsonCommentHandling.Skip, + AllowTrailingCommas = true, + UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip, + Converters = { new JsonStringEnumConverter() }, + }; + + /// The serializer options, exposed so tests can assert the enum guard on a real round-trip. + internal static JsonSerializerOptions JsonOptionsForTest => JsonOptions; + + /// + /// Registers the Sql factory with the driver registry. The optional + /// is captured at registration time and used to build a logger per + /// driver instance; without it the driver runs with the null logger. + /// + /// The driver factory registry to register with. + /// Optional logger factory for creating loggers per driver instance. + /// is null. + public static void Register(DriverFactoryRegistry registry, ILoggerFactory? loggerFactory = null) + { + ArgumentNullException.ThrowIfNull(registry); + registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory)); + } + + /// Builds one driver instance from its config blob. For the Server bootstrapper and tests. + /// The central config DB's identity for this driver instance. + /// The instance's DriverConfig JSON blob. + /// The constructed . + public static SqlDriver CreateInstance(string driverInstanceId, string driverConfigJson) + => CreateInstance(driverInstanceId, driverConfigJson, loggerFactory: null); + + /// + /// Logger-aware overload — used by 's closure when wired through DI. + /// Constructs the driver only: the driver builds its own , because + /// the reader's resolve delegate must read the driver's live authored-tag table, which does not + /// exist until the driver does. + /// No I/O happens here. The database is first contacted by + /// , so a database that is merely down yields a driver in + /// Reconnecting rather than a deployment that cannot be constructed. + /// + /// The central config DB's identity for this driver instance. + /// The instance's DriverConfig JSON blob. + /// Optional logger factory for creating loggers per driver instance. + /// The constructed . + /// An argument is null, empty or whitespace. + /// + /// The blob is not valid JSON, omits connectionStringRef, names a provider this build cannot + /// construct, carries an invalid knob, or the referenced connection string is not provisioned. + /// + public static SqlDriver CreateInstance( + string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory) + { + ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); + ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson); + + var dto = Deserialize(driverInstanceId, driverConfigJson); + + if (string.IsNullOrWhiteSpace(dto.ConnectionStringRef)) + { + throw new InvalidOperationException( + $"Sql driver config for '{driverInstanceId}' is missing the required connectionStringRef. " + + $"Author the NAME of a connection string (the value itself is supplied out-of-band through " + + $"the '{SqlConnectionStringResolver.EnvironmentVariablePrefix}' environment variable)."); + } + + var dialect = CreateDialect(dto.Provider, driverInstanceId); + + // Resolved BEFORE the knob validation below, so that every subsequent throw is a path that HAS the + // secret in hand — which is exactly where a careless interpolation would leak it. + var connectionString = SqlConnectionStringResolver.Resolve(dto.ConnectionStringRef!); + var options = BuildOptions(dto, driverInstanceId); + + var logger = (ILogger?)loggerFactory?.CreateLogger(typeof(SqlDriverFactoryExtensions).FullName!) + ?? NullLogger.Instance; + + if (dto.AllowWrites is true) + { + // Inert by construction (SqlDriver does not implement IWritable), so failing the driver over it + // would brick a deployment for a flag that cannot do harm. The operator who believes writes are + // on, however, can — so this is loud. + logger.LogWarning( + "Sql driver {DriverInstanceId} authored allowWrites=true, which this build ignores: the Sql " + + "driver is read-only (it implements no write capability), so no configuration can enable a " + + "write. Remove the flag, or expect reads only.", + driverInstanceId); + } + + var driver = new SqlDriver( + options, + driverInstanceId, + dialect, + connectionString, + factory: null, + logger: loggerFactory?.CreateLogger()); + + // Endpoint is the credential-free server/database rendering — the ONLY form of the connection string + // permitted outside the provider call. + logger.LogInformation( + "Sql driver {DriverInstanceId} configured for {Provider} at {Endpoint} with {TagCount} " + + "authored tag(s), poll {PollInterval}, operation timeout {OperationTimeout}.", + driverInstanceId, dto.Provider, driver.Endpoint, options.RawTags.Count, + options.DefaultPollInterval, options.OperationTimeout); + + return driver; + } + + /// Deserialises the blob, turning a malformed one into an actionable, instance-named failure. + private static SqlDriverConfigDto Deserialize(string driverInstanceId, string driverConfigJson) + { + try + { + return JsonSerializer.Deserialize(driverConfigJson, JsonOptions) + ?? throw new InvalidOperationException( + $"Sql driver config for '{driverInstanceId}' deserialised to null"); + } + catch (JsonException ex) + { + throw new InvalidOperationException( + $"Sql driver config for '{driverInstanceId}' is not valid JSON: {ex.Message}", ex); + } + } + + /// + /// Selects the provider seam. + /// v1 constructs only. The other members exist on the shipped + /// enum as reserved names; authoring one is a config mistake that must fail at construction with a + /// message that says so, rather than silently degrading to T-SQL against a non-SQL-Server database. + /// + private static ISqlDialect CreateDialect(SqlProvider provider, string driverInstanceId) + => provider switch + { + SqlProvider.SqlServer => new SqlServerDialect(), + _ => throw new InvalidOperationException( + $"Sql driver config for '{driverInstanceId}' names provider '{provider}', which is not " + + $"available in this build. Only '{nameof(SqlProvider.SqlServer)}' is supported."), + }; + + /// + /// Applies the documented defaults and validates every knob (design §5.1 / §8.3). + /// Internal so the defaults and the rejections can be asserted directly, without a database or a + /// provisioned connection string standing between the test and the rule. + /// + /// The deserialised config blob. + /// Named in every rejection message, so a fleet-wide log identifies the row. + /// The typed options the driver is constructed with. + /// A knob is out of range, or the two deadlines are inverted. + internal static SqlDriverOptions BuildOptions(SqlDriverConfigDto dto, string driverInstanceId) + { + ArgumentNullException.ThrowIfNull(dto); + + var defaultPollInterval = dto.DefaultPollInterval ?? TimeSpan.FromSeconds(5); + var operationTimeout = dto.OperationTimeout ?? TimeSpan.FromSeconds(15); + var commandTimeout = dto.CommandTimeout ?? TimeSpan.FromSeconds(10); + var maxConcurrentGroups = dto.MaxConcurrentGroups ?? 4; + + RequirePositive(defaultPollInterval, "defaultPollInterval", driverInstanceId); + RequirePositive(operationTimeout, "operationTimeout", driverInstanceId); + RequirePositive(commandTimeout, "commandTimeout", driverInstanceId); + if (maxConcurrentGroups < 1) + { + throw new InvalidOperationException( + $"Sql driver config for '{driverInstanceId}' has maxConcurrentGroups {maxConcurrentGroups}; " + + $"it must be at least 1."); + } + + // Design §8.3. The two deadlines are independent and BOTH are required: commandTimeout is the + // server-side backstop, operationTimeout the client-side wall-clock bound that alone survives a + // wedged socket (the R2-01/STAB-14 lesson). Inverted — or equal — the client-side abort always fires + // first and the backstop can never be reached, so a deployment quietly runs on one bound instead of + // two. Nothing downstream enforces this: the reader stays deliberately usable with it inverted. + if (operationTimeout <= commandTimeout) + { + throw new InvalidOperationException( + $"Sql driver config for '{driverInstanceId}' has operationTimeout {operationTimeout} which " + + $"is not greater than commandTimeout {commandTimeout}. The client-side operationTimeout must " + + $"be strictly greater than the server-side commandTimeout backstop, or the backstop can " + + $"never fire."); + } + + return new SqlDriverOptions + { + RawTags = dto.RawTags is { Count: > 0 } rawTags ? [.. rawTags] : [], + DefaultPollInterval = defaultPollInterval, + OperationTimeout = operationTimeout, + CommandTimeout = commandTimeout, + MaxConcurrentGroups = maxConcurrentGroups, + NullIsBad = dto.NullIsBad ?? false, + }; + } + + /// Rejects a non-positive duration, naming the authored field. + private static void RequirePositive(TimeSpan value, string field, string driverInstanceId) + { + if (value <= TimeSpan.Zero) + { + throw new InvalidOperationException( + $"Sql driver config for '{driverInstanceId}' has {field} {value}; it must be greater than zero."); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs new file mode 100644 index 00000000..0fb8fdb0 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs @@ -0,0 +1,452 @@ +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; + +/// +/// Proves the composition layer: (config blob → live driver) and +/// (a connectionStringRef name → the secret it stands for). +/// Three things here are behaviour rather than plumbing, and each has its own test below. +/// (1) The string-enum guard — 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 provider and write the +/// NAME. (2) Config validation — operationTimeout > commandTimeout 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. (3) Credential hygiene — 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. +/// +public sealed class SqlDriverFactoryTests +{ + // ---- the plan's headline legs ---- + + [Fact] + public void CreateInstance_missingConnectionStringRef_throwsActionable() + => Should.Throw(() => + 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(() => 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(() => SqlConnectionStringResolver.Resolve(name)); + } + + [Fact] + public void CreateInstance_whenTheRefResolvesToNothing_namesTheEnvironmentVariable_notTheRef() + { + var name = NewRef(); + + var ex = Should.Throw(() => 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(() => 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(() => 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(() => 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(() => 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( + """ + { + "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(); + 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(() => 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(); + 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(() => + SqlDriverFactoryExtensions.CreateInstance(blank, """{"connectionStringRef":"x"}""", null)); + Should.Throw(() => SqlDriverFactoryExtensions.CreateInstance("d1", blank, null)); + } + + [Fact] + public void CreateInstance_configThatIsNotJson_throwsNamingTheDriverInstance() + { + var ex = Should.Throw( + () => SqlDriverFactoryExtensions.CreateInstance("d1", "not json at all", null)); + + ex.Message.ShouldContain("d1"); + } + + /// A per-test connection-string ref, so no two tests can collide over one process-wide env var. + private static string NewRef() => $"OtOpcUaSqlTest{Guid.NewGuid():N}"; +} + +/// +/// Sets one environment variable for the life of the scope and restores its previous value — including +/// restoring it to . 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. +/// +internal sealed class EnvironmentVariableScope : IDisposable +{ + private readonly string _name; + private readonly string? _previous; + + /// Sets to , remembering what was there. + /// The environment variable to set. + /// The value to set it to. + public EnvironmentVariableScope(string name, string? value) + { + _name = name; + _previous = Environment.GetEnvironmentVariable(name); + Environment.SetEnvironmentVariable(name, value); + } + + /// Restores the previous value. + public void Dispose() => Environment.SetEnvironmentVariable(_name, _previous); +} + +/// +/// Captures what the factory logged, so "warned" and "did not leak the connection string" are asserted +/// rather than assumed. +/// +internal sealed class RecordingLoggerFactory : ILoggerFactory +{ + private readonly List _entries = []; + + /// The log entries recorded so far, in order. + public IReadOnlyList Entries + { + get { lock (_entries) return [.. _entries]; } + } + + /// + public ILogger CreateLogger(string categoryName) => new Recorder(this); + + /// + public void AddProvider(ILoggerProvider provider) { } + + /// + public void Dispose() { } + + private void Record(LogLevel level, string message) + { + lock (_entries) _entries.Add(new LogEntry(level, message)); + } + + /// One captured log entry. + /// The level it was logged at. + /// The formatted message. + internal sealed record LogEntry(LogLevel Level, string Message); + + private sealed class Recorder(RecordingLoggerFactory owner) : ILogger + { + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => owner.Record(logLevel, formatter(state, exception)); + } + + private sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + + public void Dispose() { } + } +}