diff --git a/.github/workflows/critical-vuln-check.yml b/.github/workflows/critical-vuln-check.yml index 61aa59f6..70f912b7 100644 --- a/.github/workflows/critical-vuln-check.yml +++ b/.github/workflows/critical-vuln-check.yml @@ -22,7 +22,7 @@ run-name: vuln-check-${{ github.event.pull_request.number }} on: pull_request_target: - branches: [releases/r8.0, develop] + branches: [releases/r10.0, releases/r8.0, develop] # critical-vuln-gate.yml's own job requests these two scopes -- a caller can # only narrow permissions for a nested reusable-workflow job, never widen diff --git a/SW.Bitween.Api/Services/RunFlagUpdater.cs b/SW.Bitween.Api/Services/RunFlagUpdater.cs index 5a1094d7..4b377d87 100644 --- a/SW.Bitween.Api/Services/RunFlagUpdater.cs +++ b/SW.Bitween.Api/Services/RunFlagUpdater.cs @@ -21,21 +21,21 @@ public async Task MarkAsRunning(int id) { var sqlUpdate = _dbType.ToLower() switch { - "pgsql" => $@"UPDATE infolink.subscription SET is_running = true - WHERE id = '{id}' and is_running = false + "pgsql" => @"UPDATE infolink.subscription SET is_running = true + WHERE id = {0} and is_running = false RETURNING is_running", - "mssql" => $@"UPDATE Subscriptions SET IsRunning = 1 + "mssql" => @"UPDATE Subscriptions SET IsRunning = 1 OUTPUT INSERTED.IsRunning - WHERE Id = '{id}' and IsRunning = 0", - "mysql" => $@"SELECT IsRunning FROM Subscriptions - WHERE Id = '{id}' and IsRunning = false + WHERE Id = {0} and IsRunning = 0", + "mysql" => @"SELECT IsRunning FROM Subscriptions + WHERE Id = {0} and IsRunning = false FOR UPDATE; UPDATE Subscriptions SET IsRunning = true - WHERE Id = '{id}' and IsRunning = false", + WHERE Id = {0} and IsRunning = false", _ => "" }; - var results = await dbContext.Set().FromSqlRaw(sqlUpdate).ToListAsync(); + var results = await dbContext.Set().FromSqlRaw(sqlUpdate, id).ToListAsync(); var result = results.SingleOrDefault(); // result is null when is running is true @@ -46,17 +46,17 @@ public async Task MarkAsIdle(int id) { var sqlUpdate = _dbType.ToLower() switch { - "pgsql" => $@"UPDATE infolink.subscription SET is_running = false - WHERE id = '{id}'", - "mssql" => $@"UPDATE Subscriptions SET IsRunning = 0 - WHERE Id = '{id}'", - "mysql" => $@"UPDATE Subscriptions SET IsRunning = false - WHERE Id = '{id}'", + "pgsql" => @"UPDATE infolink.subscription SET is_running = false + WHERE id = {0}", + "mssql" => @"UPDATE Subscriptions SET IsRunning = 0 + WHERE Id = {0}", + "mysql" => @"UPDATE Subscriptions SET IsRunning = false + WHERE Id = {0}", _ => "" }; ; - await dbContext.Database.ExecuteSqlRawAsync(sqlUpdate); + await dbContext.Database.ExecuteSqlRawAsync(sqlUpdate, id); } public class RunningResult diff --git a/SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs b/SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs new file mode 100644 index 00000000..079b85f8 --- /dev/null +++ b/SW.Bitween.IntegrationTests/Tests/RunFlagUpdaterTests.cs @@ -0,0 +1,72 @@ +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using SW.Bitween.Domain; +using SW.Bitween.IntegrationTests.Fixtures; +using SW.Bitween.Model; +using Xunit; + +namespace SW.Bitween.IntegrationTests.Tests; + +// The run flag is written with raw (parameterized) SQL that differs per database +// provider, so it needs a real round trip to prove the statement and its parameter +// binding are correct. There was no coverage here before. +[Collection("Bitween")] +public class RunFlagUpdaterTests +{ + private readonly BitweenFixture _fixture; + + public RunFlagUpdaterTests(BitweenFixture fixture) + { + _fixture = fixture; + } + + [Fact] + public async Task Run_flag_claims_once_then_blocks_until_idle() + { + await using var scope = _fixture.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var runFlag = scope.ServiceProvider.GetRequiredService(); + + var document = new Document(6101, "Run Flag Test Doc"); + db.Set().Add(document); + var subscription = new Subscription("Run Flag Test", document.Id); + subscription.Inactive = false; + db.Set().Add(subscription); + await db.SaveChangesAsync(); + + Assert.True(await runFlag.MarkAsRunning(subscription.Id)); // first claim wins + Assert.False(await runFlag.MarkAsRunning(subscription.Id)); // already running + + await runFlag.MarkAsIdle(subscription.Id); + + Assert.True(await runFlag.MarkAsRunning(subscription.Id)); // claimable again + await runFlag.MarkAsIdle(subscription.Id); + } + + // Guards the parameter binding: a broken placeholder would either match no rows + // or every row, and both would show up here. + [Fact] + public async Task Run_flag_only_affects_the_requested_subscription() + { + await using var scope = _fixture.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var runFlag = scope.ServiceProvider.GetRequiredService(); + + var document = new Document(6102, "Run Flag Isolation Doc"); + db.Set().Add(document); + var a = new Subscription("Run Flag A", document.Id); + a.Inactive = false; + var b = new Subscription("Run Flag B", document.Id); + b.Inactive = false; + db.Set().AddRange(a, b); + await db.SaveChangesAsync(); + + Assert.True(await runFlag.MarkAsRunning(a.Id)); + Assert.True(await runFlag.MarkAsRunning(b.Id)); // b untouched by a's update + + await runFlag.MarkAsIdle(a.Id); + Assert.False(await runFlag.MarkAsRunning(b.Id)); // b still running, a's idle did not clear it + + await runFlag.MarkAsIdle(b.Id); + } +}