Match whole segments when marking a build as updating, and count shared request keys [patch]

BuildMonitor.Test/ActiveRequestTests.cs

@@ -0,0 +1,52 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.BuildMonitor.Test;
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+/// <summary>
+/// Covers how in-flight requests mark a build as updating (ktsu-dev/BuildMonitor#351): a build's key
+/// has to match whole segments of a request key, and a key shared by two concurrent requests has to
+/// stay until both finish.
+/// </summary>
+[TestClass]
+public sealed class ActiveRequestTests
+{
+	private const string BuildKey = "GitHub/ktsu-dev/BuildMonitor/Build";
+
+	[TestMethod]
+	public void ABuildMatchesItsOwnPollAndItsRunsPolls()
+	{
+		Assert.IsTrue(BuildMonitor.IsRequestForBuild(BuildKey, BuildKey));
+		Assert.IsTrue(BuildMonitor.IsRequestForBuild($"{BuildKey}/Build #42", BuildKey));
+	}
+
+	[TestMethod]
+	public void ABuildDoesNotMatchAnotherBuildItsNameIsAPrefixOf()
+	{
+		Assert.IsFalse(BuildMonitor.IsRequestForBuild($"{BuildKey} and Release", BuildKey));
+		Assert.IsFalse(BuildMonitor.IsRequestForBuild($"{BuildKey} and Release/Build and Release", BuildKey));
+		Assert.IsFalse(BuildMonitor.IsRequestForBuild($"{BuildKey}Docs", BuildKey));
+	}
+
+	[TestMethod]
+	public async Task AKeySharedByTwoRequestsStaysUntilBothFinish()
+	{
+		// A unique key, because ActiveRequests is process-wide and other tests may run alongside.
+		string key = $"{BuildKey}/{Guid.NewGuid():N}";
+		TaskCompletionSource first = new(TaskCreationOptions.RunContinuationsAsynchronously);
+		TaskCompletionSource second = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+		Task firstRequest = BuildMonitor.MakeRequestAsync(key, () => first.Task);
+		Task secondRequest = BuildMonitor.MakeRequestAsync(key, () => second.Task);
+		Assert.IsTrue(BuildMonitor.ActiveRequests.ContainsKey(key));
+
+		first.SetResult();
+		await firstRequest.ConfigureAwait(false);
+		Assert.IsTrue(BuildMonitor.ActiveRequests.ContainsKey(key), "The second request is still in flight.");
+
+		second.SetResult();
+		await secondRequest.ConfigureAwait(false);
+		Assert.IsFalse(BuildMonitor.ActiveRequests.ContainsKey(key));
+	}
+}

BuildMonitor/BuildMonitor.cs

@@ -33,7 +33,12 @@
 
 	private static Task UpdateTask { get; set; } = Task.CompletedTask;
 
-	internal static ConcurrentDictionary<string, DateTimeOffset> ActiveRequests { get; set; } = [];
+	/// <summary>
+	/// Gets or sets the in-flight requests, each key with the number of requests running under it.
+	/// Two ongoing runs of one GitHub workflow poll under the same key, so a key stays while any of
+	/// its requests is still running.
+	/// </summary>
+	internal static ConcurrentDictionary<string, int> ActiveRequests { get; set; } = [];
 
 	private static void Main()
 	{
@@ -186,7 +191,7 @@
 	/// fields actually occupy tests the bug itself, so a fixed binding is recognised as fixed and
 	/// anything else is refused rather than guessed at.
 	/// </remarks>
 	private static unsafe int? ProbeNativeImGuiTableColumnSize()
 	{
 		int csharpSize = sizeof(ImGuiTableColumn);
 		int widthGivenOffset;
@@ -285,7 +290,7 @@
 		return narrowFieldBytes == NarrowlyBoundColumnFields.Length * 2 ? csharpSize : null;
 	}
 
 	private static unsafe void SaveColumnWidth(string columnName, int columnIndex)
 	{
 		// Null means the layout probe rejected the binding, so there is no address we can trust.
 		if (NativeImGuiTableColumnSize is not int nativeStructSize)
@@ -412,7 +417,7 @@
 		SaveSettingsIfRequired();
 	}
 
 	private static void UpdateOwnerTabs()
 	{
 		// Build expected tabs based on provider type:
 		// - GitHub: one tab per owner (org/user)
@@ -421,7 +426,7 @@
 		HashSet<string> expectedTabIds = [AllOwnersTabId, LogsTabId];
 		Dictionary<string, (string Label, Action Content)> tabsToCreate = [];
 
 		foreach ((BuildProviderName providerName, BuildProvider provider) in AppData.BuildProviders)
 		{
 			if (provider is GitHub)
 			{
@@ -628,7 +633,7 @@
 		return displayName;
 	}
 
 	private static string MakeRepositoryDisplayName(Build build)
 	{
 		string repoName = build.Repository.Name;
 		string ownerName = build.Owner.Name;
@@ -1410,10 +1415,19 @@
 
 	private static bool IsBuildUpdating(Build build)
 	{
-		string prefix = $"{build.Repository.Owner.BuildProvider.Name}/{build.Owner.Name}/{build.Repository.Name}/{build.Name}";
-		return ActiveRequests.Keys.Any(k => k.StartsWithOrdinal(prefix));
+		string buildKey = $"{build.Repository.Owner.BuildProvider.Name}/{build.Owner.Name}/{build.Repository.Name}/{build.Name}";
+		return ActiveRequests.Keys.Any(k => IsRequestForBuild(k, buildKey));
 	}
 
+	/// <summary>
+	/// Whether a request key is the build's own poll or one of its runs' polls. Request keys end in
+	/// <c>/{build.Name}</c> or <c>/{build.Name}/{run.Name}</c>, so the build key has to match a whole
+	/// segment: a bare prefix test let a build named <c>Build</c> light up while
+	/// <c>Build and Release</c> was being polled.
+	/// </summary>
+	internal static bool IsRequestForBuild(string requestKey, string buildKey) =>
+		string.Equals(requestKey, buildKey, StringComparison.Ordinal) || requestKey.StartsWithOrdinal(buildKey + "/");
+
 	private static ImColor GetStatusColor(RunStatus status)
 	{
 		return status switch
@@ -1688,14 +1702,31 @@
 
 	internal static async Task MakeRequestAsync(string name, Func<Task> action)
 	{
-		_ = ActiveRequests.TryAdd(name, DateTimeOffset.UtcNow);
+		_ = ActiveRequests.AddOrUpdate(name, 1, (_, count) => count + 1);
 		try
 		{
 			await action.Invoke().ConfigureAwait(false);
 		}
 		finally
 		{
-			_ = ActiveRequests.TryRemove(name, out _);
+			ReleaseRequest(name);
+		}
+	}
+
+	private static void ReleaseRequest(string name)
+	{
+		// Compare-and-swap, so a request starting under the same key between the read and the write
+		// is never lost: the last one to finish is the one that removes the key.
+		while (ActiveRequests.TryGetValue(name, out int count))
+		{
+			bool released = count <= 1
+				? ActiveRequests.TryRemove(KeyValuePair.Create(name, count))
+				: ActiveRequests.TryUpdate(name, count - 1, count);
+
+			if (released)
+			{
+				return;
+			}
 		}
 	}
 }