16f401b9e7
SqlPollReader's SemaphoreSlim was never disposed and a new reader is built per rebind. The reader is now IDisposable with a reference count (owner + one per in-flight group): the gate is disposed only when the last group - including a timed-out one still wedged in the provider - releases it, so nothing ever releases into a disposed semaphore. ApplyBinding disposes the replaced reader; the final DisposeAsync disposes the live one (Shutdown does not, since a re-init without a rebind keeps polling through it). Refs #744 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
158 lines
7.2 KiB
C#
158 lines
7.2 KiB
C#
using Shouldly;
|
|
using Xunit;
|
|
using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
|
|
|
|
namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests;
|
|
|
|
/// <summary>
|
|
/// Gitea #516 — Sql was excluded from the first pass because it derives MORE than options from config
|
|
/// (its <see cref="ISqlDialect"/> and its resolved connection string are both config-derived), so
|
|
/// adopting a new <c>SqlDriverOptions</c> alone would poll a NEW tag set through the OLD database.
|
|
/// A half-applied config change is worse than a discarded one, because it looks like it worked.
|
|
/// <para>The fix is <c>ParseBinding</c> + <c>ApplyBinding</c>: one parse produces all three, and they are
|
|
/// adopted together. These tests pin the ATOMICITY, not just that a re-parse happens — that is the
|
|
/// property that made this driver special.</para>
|
|
/// </summary>
|
|
[Trait("Category", "Unit")]
|
|
public sealed class SqlReinitConfigAdoptionTests
|
|
{
|
|
private const string RefA = "SqlReinitTestA";
|
|
private const string RefB = "SqlReinitTestB";
|
|
|
|
/// <summary>
|
|
/// A reinitialize that changes <c>connectionStringRef</c> must move the driver onto the NEW database.
|
|
/// <c>Endpoint</c> is the credential-free rendering of the live connection string, so it is a direct
|
|
/// read of which connection the driver actually adopted — no log-string matching.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task Reinitialize_adopts_a_changed_connection_string_not_just_the_options()
|
|
{
|
|
using var _ = new ScopedEnvironmentVariable(
|
|
SqlConnectionStringResolver.EnvironmentVariablePrefix + RefA,
|
|
"Server=old-server;Database=OldDb;Integrated Security=true");
|
|
using var __ = new ScopedEnvironmentVariable(
|
|
SqlConnectionStringResolver.EnvironmentVariablePrefix + RefB,
|
|
"Server=new-server;Database=NewDb;Integrated Security=true");
|
|
|
|
var driver = SqlDriverFactoryExtensions.CreateInstance(
|
|
"sql-1", $$"""{"provider":"SqlServer","connectionStringRef":"{{RefA}}"}""", loggerFactory: null);
|
|
driver.Endpoint.ShouldContain("old-server");
|
|
|
|
// The database is unreachable, so init faults — irrelevant here. The assertion is which database it
|
|
// tried, i.e. whether the connection string moved with the options.
|
|
try
|
|
{
|
|
await driver.ReinitializeAsync(
|
|
$$"""{"provider":"SqlServer","connectionStringRef":"{{RefB}}"}""", CancellationToken.None);
|
|
}
|
|
catch
|
|
{
|
|
// Liveness failure against a non-existent server is expected.
|
|
}
|
|
|
|
driver.Endpoint.ShouldContain(
|
|
"new-server",
|
|
customMessage:
|
|
"SqlDriver adopted a reinitialized config but kept its ORIGINAL connection string — the exact "
|
|
+ "half-applied adoption (#516) that would poll a new tag set against the old database");
|
|
driver.Endpoint.ShouldNotContain("old-server");
|
|
}
|
|
|
|
/// <summary>
|
|
/// A config whose <c>connectionStringRef</c> names an unprovisioned environment variable must make
|
|
/// the whole reinitialize FAIL, leaving the previous binding intact — never a partial adoption where
|
|
/// new options are live against the old connection.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task A_reinitialize_that_cannot_resolve_its_connection_leaves_the_previous_binding_intact()
|
|
{
|
|
using var _ = new ScopedEnvironmentVariable(
|
|
SqlConnectionStringResolver.EnvironmentVariablePrefix + RefA,
|
|
"Server=old-server;Database=OldDb;Integrated Security=true");
|
|
|
|
var driver = SqlDriverFactoryExtensions.CreateInstance(
|
|
"sql-1", $$"""{"provider":"SqlServer","connectionStringRef":"{{RefA}}"}""", loggerFactory: null);
|
|
|
|
await Should.ThrowAsync<Exception>(() => driver.ReinitializeAsync(
|
|
"""{"provider":"SqlServer","connectionStringRef":"SqlReinitTestNeverProvisioned"}""",
|
|
CancellationToken.None));
|
|
|
|
driver.Endpoint.ShouldContain(
|
|
"old-server",
|
|
customMessage:
|
|
"a reinitialize that could not resolve its new connection must leave the previous binding whole");
|
|
}
|
|
|
|
/// <summary>An empty/placeholder document keeps the constructor-supplied binding, so the many lifecycle
|
|
/// tests that pass <c>"{}"</c> keep meaning what they meant.</summary>
|
|
[Fact]
|
|
public async Task Reinitialize_with_an_empty_document_keeps_the_constructor_binding()
|
|
{
|
|
using var _ = new ScopedEnvironmentVariable(
|
|
SqlConnectionStringResolver.EnvironmentVariablePrefix + RefA,
|
|
"Server=old-server;Database=OldDb;Integrated Security=true");
|
|
|
|
var driver = SqlDriverFactoryExtensions.CreateInstance(
|
|
"sql-1", $$"""{"provider":"SqlServer","connectionStringRef":"{{RefA}}"}""", loggerFactory: null);
|
|
|
|
try { await driver.ReinitializeAsync("{}", CancellationToken.None); }
|
|
catch { /* liveness failure expected */ }
|
|
|
|
driver.Endpoint.ShouldContain("old-server");
|
|
}
|
|
|
|
/// <summary>
|
|
/// <b>R06:S-3.</b> Every rebind builds a new <see cref="SqlPollReader"/>; the replaced one (and its
|
|
/// <c>SemaphoreSlim</c>) used to be dropped undisposed, one per reinitialize. The control: the reader
|
|
/// that replaced it is live.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task A_rebind_disposes_the_reader_it_replaces()
|
|
{
|
|
using var _ = new ScopedEnvironmentVariable(
|
|
SqlConnectionStringResolver.EnvironmentVariablePrefix + RefA,
|
|
"Server=old-server;Database=OldDb;Integrated Security=true");
|
|
using var __ = new ScopedEnvironmentVariable(
|
|
SqlConnectionStringResolver.EnvironmentVariablePrefix + RefB,
|
|
"Server=new-server;Database=NewDb;Integrated Security=true");
|
|
|
|
var driver = SqlDriverFactoryExtensions.CreateInstance(
|
|
"sql-1", $$"""{"provider":"SqlServer","connectionStringRef":"{{RefA}}"}""", loggerFactory: null);
|
|
var original = driver.ReaderForTests;
|
|
|
|
try
|
|
{
|
|
await driver.ReinitializeAsync(
|
|
$$"""{"provider":"SqlServer","connectionStringRef":"{{RefB}}"}""", CancellationToken.None);
|
|
}
|
|
catch
|
|
{
|
|
// Liveness failure against a non-existent server is expected; the rebind ran first.
|
|
}
|
|
|
|
driver.ReaderForTests.ShouldNotBeSameAs(original);
|
|
original.GateDisposedForTests.ShouldBeTrue();
|
|
driver.ReaderForTests.GateDisposedForTests.ShouldBeFalse();
|
|
|
|
await driver.DisposeAsync();
|
|
driver.ReaderForTests.GateDisposedForTests.ShouldBeTrue("the final dispose releases the live reader too");
|
|
}
|
|
|
|
/// <summary>Sets an environment variable for the duration of a test and restores it after. The resolver
|
|
/// reads process-global state, so a leaked variable would race every sibling test.</summary>
|
|
private sealed class ScopedEnvironmentVariable : IDisposable
|
|
{
|
|
private readonly string _name;
|
|
private readonly string? _previous;
|
|
|
|
public ScopedEnvironmentVariable(string name, string value)
|
|
{
|
|
_name = name;
|
|
_previous = Environment.GetEnvironmentVariable(name);
|
|
Environment.SetEnvironmentVariable(name, value);
|
|
}
|
|
|
|
public void Dispose() => Environment.SetEnvironmentVariable(_name, _previous);
|
|
}
|
|
}
|