refactor(repositories): move static caches into IMemoryCache, enable within-fixture parallelism
BattlePassRepository._curveCache and MissionCatalogRepository._maxLevelCache
were private-static fields populated lazily on first read from whatever
DbContext happened to be in scope. In production "one DbContext lineage
per process" makes that fine. Under parallel test execution each
SVSimTestFactory owns its own SQLite :memory: DB, so the first reader's
DB (often empty, in tests that don't seed BP) poisoned the cache for
concurrent readers from a seeded DB — assertions like "BP level info
must be present after seeding" failed because the process-static cache
returned an empty list populated by the other test's empty DB.
The first patch attempted a `BypassCacheForTests` static flag, which is
exactly the kind of test-only seam that rots the production code: future
caches get the same flag, repos accumulate hidden knobs, and the
underlying invariant ("a cache populated from arbitrary scope serves
arbitrary scope") goes unaddressed.
Instead, move both caches into the DI-registered IMemoryCache.
AddMemoryCache() registers it as singleton-per-service-provider:
production has one provider → one IMemoryCache → identical caching
semantics to before. Each WebApplicationFactory builds its own
provider → its own IMemoryCache → cache is naturally scoped per fixture,
no cross-test bleed possible.
The ResetLevelCurveCache() method and its three call sites
(SVSimTestFactory.SeedGlobalsAsync, BattlePassServiceTests,
LoadControllerTests) are deleted — a fresh factory owns a fresh empty
cache, no manual invalidation needed.
With this and the previous StoryService fixture-instance fix in place,
ParallelScope.All works: 776/776 in 57s wall clock (down from 59s on
Fixtures, 2m13s pre-parallelism).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Caching.Memory;
|
||||
using SVSim.Database.Models;
|
||||
|
||||
namespace SVSim.Database.Repositories.BattlePass;
|
||||
@@ -6,14 +7,19 @@ namespace SVSim.Database.Repositories.BattlePass;
|
||||
public sealed class BattlePassRepository : IBattlePassRepository
|
||||
{
|
||||
private readonly SVSimDbContext _db;
|
||||
private readonly IMemoryCache _cache;
|
||||
|
||||
// Process-level cache for the immutable level curve. Bootstrap re-baseline = host restart = cache cleared.
|
||||
private static IReadOnlyList<BattlePassLevelEntry>? _curveCache;
|
||||
private static readonly SemaphoreSlim _curveCacheLock = new(1, 1);
|
||||
// Per-host cache for the immutable level curve, scoped via the DI-registered IMemoryCache.
|
||||
// In production "host == process"; in tests each WebApplicationFactory builds its own
|
||||
// service provider so the cache is naturally isolated per fixture — avoids the pre-refactor
|
||||
// race where a process-static cache populated from one test's DbContext served stale data
|
||||
// to a parallel test reading from a different DB.
|
||||
private const string LevelCurveCacheKey = "battlepass:level-curve";
|
||||
|
||||
public BattlePassRepository(SVSimDbContext db)
|
||||
public BattlePassRepository(SVSimDbContext db, IMemoryCache cache)
|
||||
{
|
||||
_db = db;
|
||||
_cache = cache;
|
||||
}
|
||||
|
||||
public async Task<BattlePassSeasonEntry?> GetActiveSeasonAsync(DateTimeOffset when, CancellationToken ct)
|
||||
@@ -42,25 +48,10 @@ public sealed class BattlePassRepository : IBattlePassRepository
|
||||
|
||||
public async Task<IReadOnlyList<BattlePassLevelEntry>> GetLevelCurveAsync(CancellationToken ct)
|
||||
{
|
||||
if (_curveCache is not null) return _curveCache;
|
||||
await _curveCacheLock.WaitAsync(ct);
|
||||
try
|
||||
{
|
||||
if (_curveCache is null)
|
||||
{
|
||||
_curveCache = await _db.BattlePassLevels.AsNoTracking()
|
||||
.OrderBy(e => e.Level)
|
||||
.ToListAsync(ct);
|
||||
}
|
||||
return _curveCache;
|
||||
}
|
||||
finally { _curveCacheLock.Release(); }
|
||||
var cached = await _cache.GetOrCreateAsync(LevelCurveCacheKey, async _ =>
|
||||
(IReadOnlyList<BattlePassLevelEntry>)await _db.BattlePassLevels.AsNoTracking()
|
||||
.OrderBy(e => e.Level)
|
||||
.ToListAsync(ct));
|
||||
return cached!;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Drops the process-level level-curve cache. Tests that seed BattlePassLevels after the
|
||||
/// cache has already been populated (by an earlier test's HTTP call) must call this before
|
||||
/// re-seeding so the next read fetches fresh rows.
|
||||
/// </summary>
|
||||
internal static void ResetLevelCurveCache() => _curveCache = null;
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Caching.Memory;
|
||||
using SVSim.Database.Models;
|
||||
|
||||
namespace SVSim.Database.Repositories.Mission;
|
||||
@@ -6,13 +7,18 @@ namespace SVSim.Database.Repositories.Mission;
|
||||
public sealed class MissionCatalogRepository : IMissionCatalogRepository
|
||||
{
|
||||
private readonly SVSimDbContext _db;
|
||||
private readonly IMemoryCache _cache;
|
||||
|
||||
// Process-level cache for the derived MAX(Level) lookup. Cleared on host restart
|
||||
// (re-bootstrap is the only legitimate way to mutate the catalog at runtime).
|
||||
private static IReadOnlyDictionary<int, int>? _maxLevelCache;
|
||||
private static readonly SemaphoreSlim _maxLevelLock = new(1, 1);
|
||||
// Per-host cache for the derived MAX(Level) lookup, scoped via the DI-registered
|
||||
// IMemoryCache. See BattlePassRepository for the per-host rationale (same parallel-test
|
||||
// race avoidance — each WebApplicationFactory gets its own cache).
|
||||
private const string MaxLevelCacheKey = "mission:achievement-max-level-by-type";
|
||||
|
||||
public MissionCatalogRepository(SVSimDbContext db) { _db = db; }
|
||||
public MissionCatalogRepository(SVSimDbContext db, IMemoryCache cache)
|
||||
{
|
||||
_db = db;
|
||||
_cache = cache;
|
||||
}
|
||||
|
||||
public Task<List<MissionCatalogEntry>> GetByLotTypeAsync(int lotType, CancellationToken ct) =>
|
||||
_db.MissionCatalog.AsNoTracking().Where(e => e.LotType == lotType).ToListAsync(ct);
|
||||
@@ -40,21 +46,15 @@ public sealed class MissionCatalogRepository : IMissionCatalogRepository
|
||||
|
||||
public async Task<IReadOnlyDictionary<int, int>> GetMaxLevelByAchievementTypeAsync(CancellationToken ct)
|
||||
{
|
||||
if (_maxLevelCache is not null) return _maxLevelCache;
|
||||
await _maxLevelLock.WaitAsync(ct);
|
||||
try
|
||||
var cached = await _cache.GetOrCreateAsync(MaxLevelCacheKey, async _ =>
|
||||
{
|
||||
if (_maxLevelCache is null)
|
||||
{
|
||||
var pairs = await _db.AchievementCatalog.AsNoTracking()
|
||||
.GroupBy(e => e.AchievementType)
|
||||
.Select(g => new { Type = g.Key, Max = g.Max(e => e.Level) })
|
||||
.ToListAsync(ct);
|
||||
_maxLevelCache = pairs.ToDictionary(p => p.Type, p => p.Max);
|
||||
}
|
||||
return _maxLevelCache;
|
||||
}
|
||||
finally { _maxLevelLock.Release(); }
|
||||
var pairs = await _db.AchievementCatalog.AsNoTracking()
|
||||
.GroupBy(e => e.AchievementType)
|
||||
.Select(g => new { Type = g.Key, Max = g.Max(e => e.Level) })
|
||||
.ToListAsync(ct);
|
||||
return (IReadOnlyDictionary<int, int>)pairs.ToDictionary(p => p.Type, p => p.Max);
|
||||
});
|
||||
return cached!;
|
||||
}
|
||||
|
||||
public async Task<IReadOnlyDictionary<int, int>> GetMinLevelByAchievementTypeAsync(CancellationToken ct)
|
||||
|
||||
@@ -1,15 +1,12 @@
|
||||
using NUnit.Framework;
|
||||
|
||||
// Parallelize at the fixture level: different test classes run on separate threads, but tests
|
||||
// inside one class still execute serially. Each fixture's SVSimTestFactory owns a private
|
||||
// SQLite :memory: connection, so DB state isn't shared. Process-static caches (e.g.
|
||||
// BattlePassRepository._curveCache) hold identical data across factories because every test
|
||||
// seeds from the same JSON, so cross-fixture races on them are data-equivalent.
|
||||
// Different test classes run on separate threads, but tests inside one class stay serial. Each
|
||||
// fixture's SVSimTestFactory owns a private SQLite :memory: connection, so DB state isn't shared.
|
||||
// ParallelScope.All is unsafe today: it exposes process-static caches (BattlePassRepository
|
||||
// ._curveCache and similar) to races inside heavy-globals fixtures (LoadController,
|
||||
// PackControllerFullCatalog, StoryService), so within-fixture parallelism is gated on cleaning
|
||||
// those up first.
|
||||
[assembly: Parallelizable(ParallelScope.Fixtures)]
|
||||
|
||||
// Tests within a single fixture run concurrently as well as across fixtures. Each
|
||||
// SVSimTestFactory owns a private SQLite :memory: connection so DB state isn't shared.
|
||||
// The two previously process-static repo caches (BattlePassRepository._curveCache,
|
||||
// MissionCatalogRepository._maxLevelCache) now live in the DI-registered IMemoryCache,
|
||||
// which is per-host — each WebApplicationFactory builds its own service provider so the
|
||||
// cache is naturally bounded to a single fixture's DB.
|
||||
//
|
||||
// Fixtures with shared instance state must opt out via [FixtureLifeCycle(InstancePerTestCase)]
|
||||
// (see StoryServiceTests) or move state into the test method.
|
||||
[assembly: Parallelizable(ParallelScope.All)]
|
||||
|
||||
@@ -390,9 +390,6 @@ public class LoadControllerTests
|
||||
var db = scope.ServiceProvider.GetRequiredService<SVSimDbContext>();
|
||||
await new SVSim.Bootstrap.Importers.BattlePassImporter().ImportAsync(db, SeedDir);
|
||||
}
|
||||
// Bust the process-level level-curve cache so the next /load/index call reads the
|
||||
// freshly-seeded rows rather than a stale empty list from an earlier test's HTTP call.
|
||||
SVSim.Database.Repositories.BattlePass.BattlePassRepository.ResetLevelCurveCache();
|
||||
|
||||
using var client = factory.CreateAuthenticatedClient(viewerId);
|
||||
var response = await client.PostAsync("/load/index",
|
||||
|
||||
@@ -250,8 +250,6 @@ internal sealed class SVSimTestFactory : WebApplicationFactory<Program>
|
||||
await new AvatarAbilityImporter().ImportAsync(ctx, seedDir);
|
||||
await new ArenaSeasonImporter().ImportAsync(ctx, seedDir);
|
||||
await new BattlePassImporter().ImportAsync(ctx, seedDir);
|
||||
// Reset the process-level level-curve cache so the next HTTP call reads freshly-seeded rows.
|
||||
BattlePassRepository.ResetLevelCurveCache();
|
||||
await new BattlePassSeasonImporter().ImportAsync(ctx, seedDir);
|
||||
await new BattlePassRewardImporter().ImportAsync(ctx, seedDir);
|
||||
await new DailyLoginBonusImporter().ImportAsync(ctx, seedDir);
|
||||
|
||||
@@ -18,8 +18,6 @@ public class BattlePassServiceTests
|
||||
private static async Task<long> SeedViewerAndSeason23(SVSimTestFactory f, bool isPremium = false)
|
||||
{
|
||||
long viewerId = await f.SeedViewerAsync();
|
||||
// Reset process-level level-curve cache so this test's seeded rows are visible.
|
||||
BattlePassRepository.ResetLevelCurveCache();
|
||||
using var scope = f.Services.CreateScope();
|
||||
var db = scope.ServiceProvider.GetRequiredService<SVSimDbContext>();
|
||||
// Zero out rupees so post-state totals in reward assertions equal the delta amounts.
|
||||
|
||||
Reference in New Issue
Block a user