From 57a6bd02c6fbe46ae169ba1d08ab2fde16179c5a Mon Sep 17 00:00:00 2001 From: cesnimda Date: Tue, 22 Sep 2026 07:33:51 +0200 Subject: [PATCH] fix: make default initialization concurrency-safe --- .../ConcurrentInitializationTests.cs | 142 ++++++++++++++++++ JobTrackerApi/Controllers/RulesController.cs | 36 +++-- .../Services/ApplicationChecklistService.cs | 20 ++- .../src/components/RulesSettingsCard.tsx | 4 + 4 files changed, 187 insertions(+), 15 deletions(-) create mode 100644 JobTrackerApi.Tests/ConcurrentInitializationTests.cs diff --git a/JobTrackerApi.Tests/ConcurrentInitializationTests.cs b/JobTrackerApi.Tests/ConcurrentInitializationTests.cs new file mode 100644 index 0000000..4b992be --- /dev/null +++ b/JobTrackerApi.Tests/ConcurrentInitializationTests.cs @@ -0,0 +1,142 @@ +using JobTrackerApi.Controllers; +using JobTrackerApi.Data; +using JobTrackerApi.Models; +using JobTrackerApi.Services; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Moq; +using Xunit; + +namespace JobTrackerApi.Tests; + +public sealed class ConcurrentInitializationTests +{ + [Fact] + public async Task Concurrent_rule_reads_share_the_single_created_settings_row() + { + await using var database = await Database.CreateAsync(); + await using var first = database.CreateContext(new SaveGate(delay: false)); + await using var second = database.CreateContext(new SaveGate(delay: true)); + + var results = await Task.WhenAll( + new RulesController(first).Get(default), + new RulesController(second).Get(default)); + + Assert.All(results, result => Assert.NotNull(result.Result)); + await using var verify = database.CreateContext(); + Assert.Single(await verify.UserRuleSettings.ToListAsync()); + } + + [Fact] + public async Task Concurrent_checklist_reads_share_the_single_default_item_set() + { + await using var database = await Database.CreateAsync(); + int jobId; + await using (var setup = database.CreateContext()) + { + var company = new Company { OwnerUserId = Database.Owner, Name = "Acme" }; + setup.Companies.Add(company); + await setup.SaveChangesAsync(); + var job = new JobApplication + { + OwnerUserId = Database.Owner, + CompanyId = company.Id, + JobTitle = "Backend Developer", + Status = "Applied", + Description = "Needs .NET and SQL.", + }; + setup.JobApplications.Add(job); + await setup.SaveChangesAsync(); + jobId = job.Id; + } + + await using var first = database.CreateContext(new SaveGate(delay: false)); + await using var second = database.CreateContext(new SaveGate(delay: true)); + var results = await Task.WhenAll( + new ApplicationChecklistService(first).GetAsync(Database.Owner, jobId, default), + new ApplicationChecklistService(second).GetAsync(Database.Owner, jobId, default)); + + var expectedCount = results[0]!.Items.Count; + Assert.Equal(expectedCount, results[1]!.Items.Count); + await using var verify = database.CreateContext(); + Assert.Equal(expectedCount, await verify.ApplicationChecklistItems.CountAsync()); + } + + private sealed class SaveGate(bool delay) : SaveChangesInterceptor + { + private static readonly object Sync = new(); + private static TaskCompletionSource _bothArrived = NewSignal(); + private static int _arrivals; + private int _used; + + public override async ValueTask> SavingChangesAsync( + DbContextEventData eventData, + InterceptionResult result, + CancellationToken cancellationToken = default) + { + if (Interlocked.Exchange(ref _used, 1) != 0) return result; + + Task wait; + lock (Sync) + { + if (_arrivals == 0 && _bothArrived.Task.IsCompleted) _bothArrived = NewSignal(); + _arrivals++; + if (_arrivals == 2) + { + _arrivals = 0; + _bothArrived.TrySetResult(true); + } + wait = _bothArrived.Task; + } + + await wait.WaitAsync(cancellationToken); + if (delay) await Task.Delay(100, cancellationToken); + return result; + } + + private static TaskCompletionSource NewSignal() => + new(TaskCreationOptions.RunContinuationsAsynchronously); + } + + private sealed class Database : IAsyncDisposable + { + public const string Owner = "user-1"; + private readonly string _path; + private readonly DbContextOptions _setupOptions; + private readonly ICurrentUserService _currentUser; + + private Database(string path) + { + _path = path; + _setupOptions = new DbContextOptionsBuilder() + .UseSqlite($"Data Source={path}") + .Options; + var currentUser = new Mock(); + currentUser.SetupGet(service => service.UserId).Returns(Owner); + _currentUser = currentUser.Object; + } + + public static async Task CreateAsync() + { + var database = new Database(Path.Combine(Path.GetTempPath(), $"jobtracker-concurrent-{Guid.NewGuid():N}.db")); + await using var db = new JobTrackerContext(database._setupOptions, database._currentUser); + await db.Database.EnsureCreatedAsync(); + return database; + } + + public JobTrackerContext CreateContext(SaveChangesInterceptor? interceptor = null) + { + var builder = new DbContextOptionsBuilder().UseSqlite($"Data Source={_path}"); + if (interceptor is not null) builder.AddInterceptors(interceptor); + return new JobTrackerContext(builder.Options, _currentUser); + } + + public ValueTask DisposeAsync() + { + SqliteConnection.ClearAllPools(); + File.Delete(_path); + return ValueTask.CompletedTask; + } + } +} diff --git a/JobTrackerApi/Controllers/RulesController.cs b/JobTrackerApi/Controllers/RulesController.cs index d7d8025..a6b7455 100644 --- a/JobTrackerApi/Controllers/RulesController.cs +++ b/JobTrackerApi/Controllers/RulesController.cs @@ -27,13 +27,7 @@ namespace JobTrackerApi.Controllers // Per-user rule settings when authenticated. if (!string.IsNullOrWhiteSpace(_db.CurrentUserId)) { - var u = await _db.UserRuleSettings.FirstOrDefaultAsync(x => x.OwnerUserId == _db.CurrentUserId, cancellationToken); - if (u is null) - { - u = new UserRuleSettings { OwnerUserId = _db.CurrentUserId }; - _db.UserRuleSettings.Add(u); - await _db.SaveChangesAsync(cancellationToken); - } + var u = await GetOrCreateUserSettingsAsync(_db.CurrentUserId, cancellationToken); return Ok(new RuleSettings { @@ -64,12 +58,7 @@ namespace JobTrackerApi.Controllers // Per-user rule settings when authenticated. if (!string.IsNullOrWhiteSpace(_db.CurrentUserId)) { - var s = await _db.UserRuleSettings.FirstOrDefaultAsync(x => x.OwnerUserId == _db.CurrentUserId, cancellationToken); - if (s is null) - { - s = new UserRuleSettings { OwnerUserId = _db.CurrentUserId }; - _db.UserRuleSettings.Add(s); - } + var s = await GetOrCreateUserSettingsAsync(_db.CurrentUserId, cancellationToken); s.AppliedFollowUpDays = Clamp(incoming.AppliedFollowUpDays, 1, 365); s.AppliedGhostDays = Clamp(incoming.AppliedGhostDays, 1, 365); @@ -101,6 +90,27 @@ namespace JobTrackerApi.Controllers return NoContent(); } + private async Task GetOrCreateUserSettingsAsync(string ownerUserId, CancellationToken cancellationToken) + { + var settings = await _db.UserRuleSettings.FirstOrDefaultAsync(x => x.OwnerUserId == ownerUserId, cancellationToken); + if (settings is not null) return settings; + + settings = new UserRuleSettings { OwnerUserId = ownerUserId }; + _db.UserRuleSettings.Add(settings); + try + { + await _db.SaveChangesAsync(cancellationToken); + return settings; + } + catch (DbUpdateException) + { + _db.Entry(settings).State = EntityState.Detached; + settings = await _db.UserRuleSettings.FirstOrDefaultAsync(x => x.OwnerUserId == ownerUserId, cancellationToken); + if (settings is null) throw; + return settings; + } + } + private static int Clamp(int v, int min, int max) => v < min ? min : v > max ? max : v; } } diff --git a/JobTrackerApi/Services/ApplicationChecklistService.cs b/JobTrackerApi/Services/ApplicationChecklistService.cs index 691b330..aa8cb5a 100644 --- a/JobTrackerApi/Services/ApplicationChecklistService.cs +++ b/JobTrackerApi/Services/ApplicationChecklistService.cs @@ -357,6 +357,7 @@ public sealed class ApplicationChecklistService : IApplicationChecklistService var missing = Defaults.Where(t => !existing.Contains(t.Key)).ToList(); if (missing.Count == 0) return items; + var seeded = new List(missing.Count); var order = 0; foreach (var template in Defaults) { @@ -377,12 +378,27 @@ public sealed class ApplicationChecklistService : IApplicationChecklistService }; _db.ApplicationChecklistItems.Add(item); items.Add(item); + seeded.Add(item); } order++; } - await _db.SaveChangesAsync(ct); - return items; + try + { + await _db.SaveChangesAsync(ct); + return items; + } + catch (DbUpdateException) + { + foreach (var item in seeded) _db.Entry(item).State = EntityState.Detached; + + var persisted = await LoadItemsAsync(ownerUserId, jobApplicationId, ct); + var persistedKeys = persisted.Where(i => i.SystemKey is not null) + .Select(i => i.SystemKey!) + .ToHashSet(StringComparer.Ordinal); + if (missing.Any(template => !persistedKeys.Contains(template.Key))) throw; + return persisted; + } } private async Task SyncAutoCompletionAsync(List items, ChecklistSignals signals, CancellationToken ct) diff --git a/job-tracker-ui/src/components/RulesSettingsCard.tsx b/job-tracker-ui/src/components/RulesSettingsCard.tsx index 563e31b..2e14bd8 100644 --- a/job-tracker-ui/src/components/RulesSettingsCard.tsx +++ b/job-tracker-ui/src/components/RulesSettingsCard.tsx @@ -25,15 +25,19 @@ export default function RulesSettingsCard() { const [loadError, setLoadError] = useState(null); useEffect(() => { + let active = true; api.get("/rules") .then((r) => { + if (!active) return; setS(r.data); setLoadError(null); }) .catch((e: any) => { + if (!active) return; setS(null); setLoadError(getApiErrorMessage(e, t("rulesLoadFailed"))); }); + return () => { active = false; }; }, [t]); const save = async () => {