diff --git a/ProjectDirector.Test/CloneTrackerTests.cs b/ProjectDirector.Test/CloneTrackerTests.cs new file mode 100644 index 0000000..bf9422e --- /dev/null +++ b/ProjectDirector.Test/CloneTrackerTests.cs @@ -0,0 +1,173 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector.Test; + +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers handing a background clone's completion back to the render thread. +/// +/// +/// The Clone button used to refresh the page from the worker thread that ran the clone, racing the +/// render thread over the repository dictionaries and the repo browser, and nothing stopped a second +/// click from starting another clone into the same folder. +/// +[TestClass] +public sealed class CloneTrackerTests +{ + private static FullyQualifiedLocalRepoPath LocalPath(string name) => + FullyQualifiedLocalRepoPath.Create(Path.Join(Path.GetTempPath(), "dev", name)); + + [TestMethod] + public void ASecondCloneIntoTheSameFolderIsRefusedWhileTheFirstRuns() + { + CloneTracker clones = new(); + + Assert.IsTrue(clones.TryStart(LocalPath("A"))); + Assert.IsTrue(clones.IsInFlight(LocalPath("A"))); + Assert.IsFalse(clones.TryStart(LocalPath("A"))); + } + + [TestMethod] + public void ACloneIntoAnotherFolderIsNotBlocked() + { + CloneTracker clones = new(); + + Assert.IsTrue(clones.TryStart(LocalPath("A"))); + Assert.IsTrue(clones.TryStart(LocalPath("B"))); + } + + [TestMethod] + public void AFolderCanBeClonedAgainOnceItsCloneCompletes() + { + CloneTracker clones = new(); + Assert.IsTrue(clones.TryStart(LocalPath("A"))); + + clones.Complete(LocalPath("A")); + + Assert.IsFalse(clones.IsInFlight(LocalPath("A"))); + Assert.IsTrue(clones.TryStart(LocalPath("A"))); + } + + [TestMethod] + public void NoRefreshIsRequestedUntilACloneCompletes() + { + CloneTracker clones = new(); + Assert.IsTrue(clones.TryStart(LocalPath("A"))); + + Assert.IsFalse(clones.TakeRefreshRequest()); + } + + [TestMethod] + public async Task ACloneCompletedOnAWorkerThreadIsRefreshedByTheCallerThatTakesTheRequestOnce() + { + CloneTracker clones = new(); + Assert.IsTrue(clones.TryStart(LocalPath("A"))); + + await Task.Run(() => clones.Complete(LocalPath("A"))).ConfigureAwait(false); + + Assert.IsTrue(clones.TakeRefreshRequest()); + Assert.IsFalse(clones.TakeRefreshRequest()); + } + + [TestMethod] + public async Task TryRunRefusesASecondCloneWhileTheFirstRunsAndRequestsARefreshWhenItEnds() + { + CloneTracker clones = new(); + using ManualResetEventSlim release = new(); + + Assert.IsTrue(clones.TryRun(LocalPath("A"), release.Wait, out Task first)); + Assert.IsTrue(clones.IsInFlight(LocalPath("A"))); + Assert.IsFalse(clones.TryRun(LocalPath("A"), () => Assert.Fail("A duplicate clone ran."), out Task refused)); + Assert.IsTrue(refused.IsCompleted); + + release.Set(); + await first.ConfigureAwait(false); + + Assert.IsFalse(clones.IsInFlight(LocalPath("A"))); + Assert.IsTrue(clones.TakeRefreshRequest()); + } + + [TestMethod] + public async Task ACloneThatThrowsIsStillRecordedAsComplete() + { + CloneTracker clones = new(); + + Assert.IsTrue(clones.TryRun(LocalPath("A"), () => throw new InvalidOperationException("clone failed"), out Task run)); + _ = await Assert.ThrowsExactlyAsync(() => run).ConfigureAwait(false); + + Assert.IsFalse(clones.IsInFlight(LocalPath("A"))); + Assert.IsTrue(clones.TakeRefreshRequest()); + } + + [TestMethod] + public void RefreshIfRequestedRefreshesOnceForACompletedClone() + { + CloneTracker clones = new(); + int refreshes = 0; + + clones.RefreshIfRequested(() => refreshes++); + Assert.AreEqual(0, refreshes); + + clones.Complete(LocalPath("A")); + clones.RefreshIfRequested(() => refreshes++); + clones.RefreshIfRequested(() => refreshes++); + + Assert.AreEqual(1, refreshes); + } + + [TestMethod] + public void MakeCloneClonesTheRemoteIntoTheFolderAndLogsTheResult() + { + string root = Path.Join(Path.GetTempPath(), $"ktsu_pd_{Guid.NewGuid():N}"); + try + { + string origin = Path.Join(root, "origin"); + string clone = Path.Join(root, "clone"); + Assert.IsTrue(GitCli.Run("init", origin).Succeeded, "git init failed."); + string? description = null; + GitResult? result = null; + + ProjectDirector.MakeClone( + GitRemotePath.Create(origin), + FullyQualifiedLocalRepoPath.Create(clone), + (d, r) => (description, result) = (d, r))(); + + Assert.AreEqual($"Cloning {origin}", description); + Assert.IsNotNull(result); + Assert.IsTrue(result.Succeeded); + Assert.IsTrue(GitCli.IsRepository(clone)); + } + finally + { + TryDeleteDirectory(root); + } + } + + private static void TryDeleteDirectory(string path) + { + try + { + // Git marks objects read-only, which blocks a plain recursive delete on Windows. + foreach (string file in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories)) + { + File.SetAttributes(file, FileAttributes.Normal); + } + + Directory.Delete(path, recursive: true); + } + catch (IOException) + { + // A best-effort cleanup of a temp directory is not worth failing a test over. + } + catch (UnauthorizedAccessException) + { + // As above. + } + } +} diff --git a/ProjectDirector/CloneTracker.cs b/ProjectDirector/CloneTracker.cs new file mode 100644 index 0000000..9cf2fce --- /dev/null +++ b/ProjectDirector/CloneTracker.cs @@ -0,0 +1,120 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector; + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +/// +/// Tracks the clones running in the background and hands their completion back to the render thread. +/// +/// +/// A clone finishes on a worker thread, and refreshing the page from there mutated +/// , and the +/// repo browser while the render thread was enumerating them. So a finished clone only records that a +/// refresh is wanted, and the tick that runs on the render thread takes the request and refreshes. +/// The same record of what is in flight keeps a second click from starting another clone into the +/// folder the first one is still writing. +/// +internal sealed class CloneTracker +{ + private readonly HashSet inFlight = []; + private readonly Lock gate = new(); + private int refreshRequested; + + /// + /// Records a clone into as started, unless one already is. + /// + /// The folder being cloned into. + /// if the caller should start the clone. + internal bool TryStart(FullyQualifiedLocalRepoPath localPath) + { + lock (gate) + { + return inFlight.Add(localPath); + } + } + + /// + /// Gets a value indicating whether a clone into is running. + /// + /// The folder to ask about. + /// while the clone runs. + internal bool IsInFlight(FullyQualifiedLocalRepoPath localPath) + { + lock (gate) + { + return inFlight.Contains(localPath); + } + } + + /// + /// Records a clone as finished, whether or not it succeeded, and asks for a refresh. Safe to call + /// from any thread. + /// + /// The folder that was cloned into. + internal void Complete(FullyQualifiedLocalRepoPath localPath) + { + lock (gate) + { + _ = inFlight.Remove(localPath); + } + + _ = Interlocked.Exchange(ref refreshRequested, 1); + } + + /// + /// Runs on the thread pool, unless a clone into + /// is already running. + /// + /// The folder being cloned into. + /// The clone itself. It must not touch UI state. + /// The running clone, or a completed task if the clone was refused. + /// if the clone was started. + /// + /// The clone is recorded as complete however it ends, so a clone that throws can be retried. + /// + internal bool TryRun(FullyQualifiedLocalRepoPath localPath, Action clone, out Task run) + { + if (!TryStart(localPath)) + { + run = Task.CompletedTask; + return false; + } + + run = Task.Run(() => + { + try + { + clone(); + } + finally + { + Complete(localPath); + } + }); + + return true; + } + + /// + /// Calls if a clone has completed since the last call. Call from the + /// render thread. + /// + /// Refreshes the page. + internal void RefreshIfRequested(Action refresh) + { + if (TakeRefreshRequest()) + { + refresh(); + } + } + + /// + /// Takes the pending refresh request, if any. Call from the render thread. + /// + /// once for each run of completions since the last call. + internal bool TakeRefreshRequest() => Interlocked.Exchange(ref refreshRequested, 0) != 0; +} diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 6a244f0..4f3b332 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -56,6 +56,11 @@ internal sealed class ProjectDirector /// private GitHubOwnerName? OwnerPendingTokenPopup { get; set; } + /// + /// The clones running in the background, whose completion the tick hands to . + /// + private CloneTracker Clones { get; } = new(); + // private ChatClient ChatClient { get; init; } private static void Main(string[] _) @@ -681,6 +686,8 @@ private void Tick(float dt) result => QueueLogIfAny(ApplyOwnerToken(owner, result))); } + Clones.RefreshIfRequested(RefreshPage); + _ = PopupSetDevDirectory.ShowIfOpen(); _ = PopupAddNewGitHubOwner.ShowIfOpen(); _ = PopupSetGitHubOwnerToken.ShowIfOpen(); @@ -703,13 +710,11 @@ private void ShowTopPanel(float dt) { if (!Options.ClonedRepos.ContainsValue(Options.BaseRepo)) { - if (ImGui.Button("Clone", new Vector2(FieldWidth, 0))) + // The button is hidden while its clone runs, and the page is refreshed by the next + // tick, on the render thread, rather than by the clone. + if (!Clones.IsInFlight(repo.LocalPath) && ImGui.Button("Clone", new Vector2(FieldWidth, 0))) { - Task.Run(() => QueueGitLog($"Cloning {repo.RemotePath}", GitCli.Run("clone", repo.RemotePath.ToString(), repo.LocalPath.ToString()))) - .ContinueWith((t) => RefreshPage(), - new CancellationToken(), - TaskContinuationOptions.OnlyOnRanToCompletion | TaskContinuationOptions.ExecuteSynchronously, - TaskScheduler.Current); + _ = Clones.TryRun(repo.LocalPath, MakeClone(repo.RemotePath, repo.LocalPath, QueueGitLog), out _); } } else @@ -794,6 +799,16 @@ private void ShowTopPanel(float dt) } } + /// + /// Makes the clone the Clone button runs in the background. + /// + /// The repository to clone. + /// The folder to clone it into. + /// Records the result. Called from the thread the clone runs on. + /// The clone, which touches no UI state. + internal static Action MakeClone(GitRemotePath remotePath, FullyQualifiedLocalRepoPath localPath, Action log) => + () => log($"Cloning {remotePath}", GitCli.Run("clone", remotePath.ToString(), localPath.ToString())); + private void RefreshPage() { if (Options.Repos.TryGetValue(Options.BaseRepo, out GitRepository? repo))