diff --git a/src/Yavsc.Org.Tests/Controllers/TestWebApplicationFactoryIsolationTests.cs b/src/Yavsc.Org.Tests/Controllers/TestWebApplicationFactoryIsolationTests.cs new file mode 100644 index 00000000..ec86ffe8 --- /dev/null +++ b/src/Yavsc.Org.Tests/Controllers/TestWebApplicationFactoryIsolationTests.cs @@ -0,0 +1,68 @@ +using IdentityServer8.EntityFramework.DbContexts; +using IdentityServer8.EntityFramework.Entities; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; + +namespace Yavsc.Org.Tests.Controllers; + +/// +/// Regression sentinel: two +/// instances must not see each other's clients. +/// +/// EF Core's UseInMemoryDatabase(name) returns the same +/// backing store to every DbContext that asks for it under +/// the same name, in the same process. Before the per-fixture GUID +/// fix, both and +/// used the bare "InMemory" +/// connection string, so every fixture shared one store and tests +/// were silently order-dependent. +/// +/// We assert against directly +/// rather than via IClientStore: the validating wrapper around +/// IClientStore raises events through IEventService, +/// which is not registered in the test host and crashes with a +/// NullReferenceException before it can return a result. Going +/// straight to the DbContext is the same code path the production +/// code uses, so it is the right surface to assert against. +/// +public class TestWebApplicationFactoryIsolationTests +{ + [Fact] + public async Task Second_factory_does_not_see_clients_seeded_into_first() + { + var marker = $"marker-A-{Guid.NewGuid():N}"; + + // First factory: seed a distinctive client. + using (var first = new TestWebApplicationFactory()) + { + await using var scope = first.Services.CreateAsyncScope(); + var configDb = scope.ServiceProvider.GetRequiredService(); + var firstCs = scope.ServiceProvider.GetRequiredService() + .GetConnectionString("YavscConnection"); + Assert.StartsWith("InMemory-", firstCs); + configDb.Clients.Add(new Client { ClientId = marker, ClientName = "marker-A" }); + await configDb.SaveChangesAsync(TestContext.Current.CancellationToken); + + // Sanity: the first factory can see its own seed. + var seenByFirst = await configDb.Clients + .AsNoTracking() + .AnyAsync(c => c.ClientId == marker, TestContext.Current.CancellationToken); + Assert.True(seenByFirst); + } + + // Second factory: must start from a clean slate. If the + // in-memory store leaked from the first factory, this + // assertion fails. + using var second = new TestWebApplicationFactory(); + await using var secondScope = second.Services.CreateAsyncScope(); + var secondCs = secondScope.ServiceProvider.GetRequiredService() + .GetConnectionString("YavscConnection"); + Assert.StartsWith("InMemory-", secondCs); + var secondDb = secondScope.ServiceProvider.GetRequiredService(); + var seenBySecond = await secondDb.Clients + .AsNoTracking() + .AnyAsync(c => c.ClientId == marker, TestContext.Current.CancellationToken); + Assert.False(seenBySecond); + } +} diff --git a/src/Yavsc.Org.Tests/TestWebApplicationFactory.cs b/src/Yavsc.Org.Tests/TestWebApplicationFactory.cs index dfd6edea..b85cba11 100644 --- a/src/Yavsc.Org.Tests/TestWebApplicationFactory.cs +++ b/src/Yavsc.Org.Tests/TestWebApplicationFactory.cs @@ -21,9 +21,54 @@ namespace Yavsc.Org.Tests; /// so that User.GetUserId() /// in user code sees a logged-in identity derived from the same /// header. +/// +/// Each instance gets its own in-memory database, identified by a +/// GUID generated in the constructor. The connection string +/// (ConnectionStrings:YavscConnection) is set as an +/// environment variable (ConnectionStrings__YavscConnection) +/// in the constructor and unset in , so the +/// production AddIdentityDBAndStores registers DbContext +/// instances against this fixture's own store. Without this, the +/// "InMemory" connection string from +/// appsettings-org.Testing.json would route every +/// instance — and any +/// running in the same process — to +/// the same backing store, leaking state between fixtures. +/// +/// Env vars are used (rather than ConfigureAppConfiguration or +/// UseSetting) because WebApplicationFactory applies +/// those too late: Program.Main has already captured the +/// connection string in AddIdentityDBAndStores by the time +/// the test host's overrides take effect. Env vars are the last +/// provider added in AddConfiguration (see +/// Yavsc.Server/Helpers/ConfigHelpers.cs), so they win. /// public class TestWebApplicationFactory : WebApplicationFactory { + private readonly string _fixtureId = Guid.NewGuid().ToString("N"); + + // ASP.NET Core's environment-variable configuration provider uses + // the key ConnectionStrings__YavscConnection (double underscore + // for the section separator). Set it before the host starts so + // the per-fixture connection string wins over + // appsettings-org.Testing.json. We do NOT touch the appsettings + // file; env vars take precedence in the configuration pipeline + // (see AddConfiguration in Yavsc.Server/Helpers/ConfigHelpers.cs, + // which adds AddEnvironmentVariables last). + private static readonly object _envLock = new(); + private bool _envSet; + + public TestWebApplicationFactory() + { + lock (_envLock) + { + Environment.SetEnvironmentVariable( + "ConnectionStrings__YavscConnection", + InMemoryDatabaseName.For(_fixtureId)); + _envSet = true; + } + } + protected override void ConfigureWebHost(IWebHostBuilder builder) { // UseEnvironment("Testing") puts the host in a dedicated @@ -50,4 +95,18 @@ public class TestWebApplicationFactory : WebApplicationFactory services.AddTransient(); }); } + + protected override void Dispose(bool disposing) + { + if (disposing && _envSet) + { + lock (_envLock) + { + Environment.SetEnvironmentVariable( + "ConnectionStrings__YavscConnection", null); + _envSet = false; + } + } + base.Dispose(disposing); + } } diff --git a/src/Yavsc.Org.Tests/WebServerFixture.cs b/src/Yavsc.Org.Tests/WebServerFixture.cs index e580438b..4f736ecf 100644 --- a/src/Yavsc.Org.Tests/WebServerFixture.cs +++ b/src/Yavsc.Org.Tests/WebServerFixture.cs @@ -42,6 +42,17 @@ public sealed class WebServerFixture : WebHostFixture { private static readonly int _httpsPort = GetAvailableLoopbackPort(); + // One in-memory database name for the whole process: WebHostFixture + // is a per-process singleton (see _app, _isInitialized, _sharedServices + // in the base class), so every WebServerFixture instance shares the + // same backing store. That is intentional — the "Yavsc Server" test + // collection groups tests that should see the same seeded state, and + // re-initialising the store per fixture would just regress the + // order-dependence we are trying to eliminate. The GUID still matters + // because TestWebApplicationFactory and WebServerFixture must not + // collide in the in-memory store; see InMemoryDatabaseName. + private static readonly string _fixtureId = Guid.NewGuid().ToString("N"); + protected override int HttpsPort => _httpsPort; private static IConfiguration? _sharedConfiguration; @@ -80,7 +91,7 @@ public sealed class WebServerFixture : WebHostFixture // that plus the in-memory overrides below. builder.AddConfiguration(null).AddInMemoryCollection(new Dictionary { - [$"ConnectionStrings:{Yavsc.Constants.YavscConnectionStringName}"] = "InMemory", + [$"ConnectionStrings:{Yavsc.Constants.YavscConnectionStringName}"] = InMemoryDatabaseName.For(_fixtureId), // SMTP test config: UserName non-null so MailSender // exercises the Authenticate branch — the // RecordingSmtpClient captures it. diff --git a/src/Yavsc.Org/Extensions/HostingExtensions.cs b/src/Yavsc.Org/Extensions/HostingExtensions.cs index f92dc3c9..6528b9c6 100644 --- a/src/Yavsc.Org/Extensions/HostingExtensions.cs +++ b/src/Yavsc.Org/Extensions/HostingExtensions.cs @@ -169,10 +169,20 @@ public static class HostingExtensions public static IdentityBuilder AddIdentityDBAndStores(this WebApplicationBuilder builder) { IServiceCollection services = builder.Services; - var connectionString = builder.Configuration.GetConnectionString(Constants.YavscConnectionStringName); - services.AddDbContext(options => + services.AddDbContext((sp, options) => { + // Read the connection string at DbContext construction time + // rather than at AddDbContext registration time, so test + // fixtures (e.g. WebApplicationFactory) can + // override the value via the host's IConfiguration before + // any DbContext is built. Reading it eagerly at the top of + // this method would freeze whatever was in configuration + // when Program.Main ran — too early for the test host's + // ConfigureAppConfiguration / UseSetting hooks to apply. + var connectionString = sp.GetRequiredService() + .GetConnectionString(Constants.YavscConnectionStringName); + if (UsesInMemoryProvider(connectionString)) { options.UseInMemoryDatabase(connectionString); @@ -317,9 +327,20 @@ public static class HostingExtensions options.ClaimsIdentity.RoleClaimType = Constants.RoleClaimType; }); var migrationsAssembly = typeof(Program).GetTypeInfo().Assembly.GetName().Name; - var connectionString = builder.Configuration.GetConnectionString(Constants.YavscConnectionStringName); - string sqliteConnectionString = $"Data Source={Path.Combine(Path.GetTempPath(), "yavsc_test.db")}"; + // The IdentityServer8.EntityFramework ConfigurationStoreOptions + // and OperationalStoreOptions expose ConfigureDbContext as an + // Action with no service-provider + // access, so the connection string has to be captured here at + // registration time. For the production runtime this is fine: + // the connection string does not change after startup. For + // tests, this is the one knob we cannot push into the per-fixture + // config pipeline; the TestWebApplicationFactory bridge instead + // sets ConnectionStrings__YavscConnection as an environment + // variable, which AddEnvironmentVariables picks up as the last + // configuration provider in AddConfiguration. See + // Yavsc.Server/Helpers/ConfigHelpers.cs. + var connectionString = builder.Configuration.GetConnectionString(Constants.YavscConnectionStringName); var identityServerBuilder = builder.Services.AddIdentityServer(options => { @@ -600,7 +621,13 @@ public static class HostingExtensions private static bool UsesInMemoryProvider(string connectionString) { - return string.Equals(connectionString, InMemoryProviderName, StringComparison.OrdinalIgnoreCase); + // Test fixtures may suffix the connection string with a + // per-fixture GUID (see InMemoryDatabaseName in + // Yavsc.Tests.Shared) to keep their in-memory stores + // isolated. The base name "InMemory" is still what + // identifies an in-memory provider — anything starting + // with it is one. + return connectionString.StartsWith(InMemoryProviderName, StringComparison.OrdinalIgnoreCase); } private static Action EnsureDefaultApplicationScopes() diff --git a/src/Yavsc.Tests.Shared/InMemoryDatabaseName.cs b/src/Yavsc.Tests.Shared/InMemoryDatabaseName.cs new file mode 100644 index 00000000..1c78c6dd --- /dev/null +++ b/src/Yavsc.Tests.Shared/InMemoryDatabaseName.cs @@ -0,0 +1,30 @@ +namespace Yavsc.Tests.Shared; + +/// +/// Helpers for the in-memory connection string used by test fixtures. +/// +/// EF Core's UseInMemoryDatabase(name) returns the same backing +/// store to every DbContext that asks for it under the same +/// , in the same process. That means every +/// fixture that uses the bare "InMemory" connection string +/// shares the same in-memory database — which leaks state between +/// fixtures that are supposed to be independent, and silently makes +/// tests order-dependent. +/// +/// The fix is to give each fixture its own suffix. +/// returns a stable, fixture-scoped connection string. The fixture +/// stores the suffix in an instance field so successive calls within +/// the same fixture always resolve to the same database. +/// +public static class InMemoryDatabaseName +{ + /// Base connection string for the in-memory provider, + /// as it appears in appsettings-org.Testing.json. + public const string Base = "InMemory"; + + /// Builds a per-fixture connection string. Two calls + /// with the same return the same + /// string; two calls with different ids return different + /// strings, isolating the underlying in-memory stores. + public static string For(string fixtureId) => $"{Base}-{fixtureId}"; +}