From 19a96417907b99ea135cd03eef18275eafcc197d Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Wed, 8 Nov 2017 14:29:03 -0500 Subject: [PATCH 1/4] Stopping watcher for git commit operations --- src/GitHub.Api/Git/RepositoryManager.cs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/GitHub.Api/Git/RepositoryManager.cs b/src/GitHub.Api/Git/RepositoryManager.cs index 0e2300a1a..853eacfc1 100644 --- a/src/GitHub.Api/Git/RepositoryManager.cs +++ b/src/GitHub.Api/Git/RepositoryManager.cs @@ -173,20 +173,20 @@ public int WaitForEvents() public ITask CommitAllFiles(string message, string body) { - var add = GitClient.AddAll(); - add.OnStart += t => IsBusy = true; - return add - .Then(GitClient.Commit(message, body)) - .Finally(() => IsBusy = false); + var task = GitClient.AddAll() + .Then(GitClient.Commit(message, body)); + + HookupHandlers(task, true); + return task; } public ITask CommitFiles(List files, string message, string body) { - var add = GitClient.Add(files); - add.OnStart += t => IsBusy = true; - return add - .Then(GitClient.Commit(message, body)) - .Finally(() => IsBusy = false); + var task = GitClient.Add(files) + .Then(GitClient.Commit(message, body)); + + HookupHandlers(task, true); + return task; } public ITask> Log() From 134558c78c8c655cc41159821f5dc6a46084bbc9 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 20 Nov 2017 16:03:02 -0500 Subject: [PATCH 2/4] Adding a task integration test that demonstrates the execution order of OnStart and OnEnd --- src/tests/TaskSystemIntegrationTests/Tests.cs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/tests/TaskSystemIntegrationTests/Tests.cs b/src/tests/TaskSystemIntegrationTests/Tests.cs index d916283ff..70a7651b8 100644 --- a/src/tests/TaskSystemIntegrationTests/Tests.cs +++ b/src/tests/TaskSystemIntegrationTests/Tests.cs @@ -108,6 +108,44 @@ public async Task ProcessReadsFromStandardInput() Assert.AreEqual(expectedOutput, output); } + [Test] + public async Task ProcessOnStartOnEndTaskOrder() + { + var values = new List(); + string process1Value = null; + string process2Value = null; + + var process1Task = new FirstNonNullLineProcessTask(Token, TestApp, @"-s 100 -d process1") + .Configure(ProcessManager, true).Then((b, s) => { + process1Value = s; + values.Add(s); + }); + + var process2Task = new FirstNonNullLineProcessTask(Token, TestApp, @"-s 100 -d process2") + .Configure(ProcessManager, true).Then((b, s) => { + process2Value = s; + values.Add(s); + }); + + var combinedTask = process1Task + .Then(process2Task); + + combinedTask.OnStart += task => { + values.Add("OnStart"); + }; + + combinedTask.OnEnd += task => { + values.Add("OnEnd"); + }; + + await combinedTask + .StartAsAsync(); + + Assert.AreEqual(process1Value, "process1"); + Assert.AreEqual(process2Value, "process2"); + Assert.True(values.SequenceEqual(new []{ "process1", "OnStart", "process2", "OnEnd" })); + } + [Test] public async Task ProcessReturningErrorThrowsException() { From 5eb059b29a56984c16e703970a1d1b84052e002d Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 20 Nov 2017 17:44:23 -0500 Subject: [PATCH 3/4] Refactored HookupHandlers to make use of the task system HookupHandlers has will disable the repository watcher and toggle the busy flag by default. Tasks that directly cause changes to the file system should toggle the stop watcher flag. The only tasks that are called from RepositoryManager and do not directly cause changes are: - GitStatusTask - GitFetchTask - GitPushTask - GitRemoteAdd - GitRemoteRemove - GitRemoteChange - GitDeleteBranch - GitCreateBranch - GitLockTask - GitUnlockTask - GitLogTask - GitListLocksTask Tasks that are exclusive or batches of exclusive operations should toggle the busy flag. The only tasks that are called from RepositoryManager and are not exclusive are: - GitLogTask - GitListLocksTask --- src/GitHub.Api/Git/RepositoryManager.cs | 92 ++++++++++++++----------- 1 file changed, 52 insertions(+), 40 deletions(-) diff --git a/src/GitHub.Api/Git/RepositoryManager.cs b/src/GitHub.Api/Git/RepositoryManager.cs index 526359293..a92abb553 100644 --- a/src/GitHub.Api/Git/RepositoryManager.cs +++ b/src/GitHub.Api/Git/RepositoryManager.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; namespace GitHub.Unity @@ -172,52 +173,50 @@ public int WaitForEvents() public ITask CommitAllFiles(string message, string body) { - var task = GitClient.AddAll() + var task = GitClient + .AddAll() .Then(GitClient.Commit(message, body)); - HookupHandlers(task, true); - return task; + return HookupHandlers(task); } public ITask CommitFiles(List files, string message, string body) { - var task = GitClient.Add(files) + var task = GitClient + .Add(files) .Then(GitClient.Commit(message, body)); - HookupHandlers(task, true); - return task; + return HookupHandlers(task); } public ITask> Log() { var task = GitClient.Log(); - HookupHandlers(task); - return task; + return HookupHandlers(task, false, false); } public ITask Status() { var task = GitClient.Status(); - HookupHandlers(task); - return task; + return HookupHandlers(task, false); } public ITask Fetch(string remote) { var task = GitClient.Fetch(remote); - return HookupHandlers(task); + return HookupHandlers(task, false); } public ITask Pull(string remote, string branch) { var task = GitClient.Pull(remote, branch); - return HookupHandlers(task, true); + return HookupHandlers(task); } public ITask Push(string remote, string branch) { var task = GitClient.Push(remote, branch); - return HookupHandlers(task); + return HookupHandlers(task, false); } public ITask Revert(string changeset) @@ -229,7 +228,7 @@ public ITask Revert(string changeset) public ITask RemoteAdd(string remote, string url) { var task = GitClient.RemoteAdd(remote, url); - HookupHandlers(task); + task = HookupHandlers(task, false); if (!platform.Environment.IsWindows) { task.Then(_ => { @@ -242,7 +241,7 @@ public ITask RemoteAdd(string remote, string url) public ITask RemoteRemove(string remote) { var task = GitClient.RemoteRemove(remote); - HookupHandlers(task); + task = HookupHandlers(task, false); if (!platform.Environment.IsWindows) { task.Then(_ => { @@ -255,44 +254,44 @@ public ITask RemoteRemove(string remote) public ITask RemoteChange(string remote, string url) { var task = GitClient.RemoteChange(remote, url); - return HookupHandlers(task); + return HookupHandlers(task, false); } public ITask SwitchBranch(string branch) { var task = GitClient.SwitchBranch(branch); - return HookupHandlers(task, true); + return HookupHandlers(task); } public ITask DeleteBranch(string branch, bool deleteUnmerged = false) { var task = GitClient.DeleteBranch(branch, deleteUnmerged); - return HookupHandlers(task); + return HookupHandlers(task, false); } public ITask CreateBranch(string branch, string baseBranch) { var task = GitClient.CreateBranch(branch, baseBranch); - return HookupHandlers(task); + return HookupHandlers(task, false); } public ITask> ListLocks(bool local) { var task = GitClient.ListLocks(local); - HookupHandlers(task); + HookupHandlers(task, false, false); return task; } public ITask LockFile(string file) { var task = GitClient.Lock(file); - return HookupHandlers(task); + return HookupHandlers(task, false); } public ITask UnlockFile(string file, bool force) { var task = GitClient.Unlock(file, force); - return HookupHandlers(task); + return HookupHandlers(task, false); } public void UpdateConfigData() @@ -329,29 +328,42 @@ private void UpdateHead() UpdateCurrentBranchAndRemote(head); } - private ITask HookupHandlers(ITask task, bool disableWatcher = false) + private ITask HookupHandlers(ITask task, bool disableWatcher = true, bool toggleBusyFlag = true) { - task.OnStart += t => { - Logger.Trace("Start " + task.Name); - IsBusy = true; + return new ActionTask(CancellationToken.None, () => { + if (toggleBusyFlag) + { + Logger.Trace("Starting Operation - Setting Busy Flag"); + IsBusy = true; + } - if (disableWatcher) - { - watcher.Stop(); - } - }; + if (disableWatcher) + { + Logger.Trace("Starting Operation - Disable Watcher"); + watcher.Stop(); + } + }) + .Then(task) + .Finally((success, exception, result) => { + if (disableWatcher) + { + Logger.Trace("Ended Operation - Enable Watcher"); + watcher.Start(); + } - task.OnEnd += t => { - if (disableWatcher) - { - watcher.Start(); - } + if (toggleBusyFlag) + { + Logger.Trace("Ended Operation - Clearing Busy Flag"); + IsBusy = false; + } - IsBusy = false; + if (success) + { + return result; + } - Logger.Trace("Finish " + task.Name); - }; - return task; + throw exception; + }); } private void Watcher_OnRemoteBranchDeleted(string remote, string name) From 8eba337076d14768f24ef3612d8a18d1c0d4df16 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Tue, 21 Nov 2017 14:06:21 -0500 Subject: [PATCH 4/4] Changing the signature of HookupHandlers to be clearer The optional arguments were killing me. --- src/GitHub.Api/Git/RepositoryManager.cs | 40 ++++++++++++------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/GitHub.Api/Git/RepositoryManager.cs b/src/GitHub.Api/Git/RepositoryManager.cs index a92abb553..db99dcbaa 100644 --- a/src/GitHub.Api/Git/RepositoryManager.cs +++ b/src/GitHub.Api/Git/RepositoryManager.cs @@ -177,7 +177,7 @@ public ITask CommitAllFiles(string message, string body) .AddAll() .Then(GitClient.Commit(message, body)); - return HookupHandlers(task); + return HookupHandlers(task, true, true); } public ITask CommitFiles(List files, string message, string body) @@ -186,7 +186,7 @@ public ITask CommitFiles(List files, string message, string body) .Add(files) .Then(GitClient.Commit(message, body)); - return HookupHandlers(task); + return HookupHandlers(task, true, true); } public ITask> Log() @@ -198,37 +198,37 @@ public ITask> Log() public ITask Status() { var task = GitClient.Status(); - return HookupHandlers(task, false); + return HookupHandlers(task, true, false); } public ITask Fetch(string remote) { var task = GitClient.Fetch(remote); - return HookupHandlers(task, false); + return HookupHandlers(task, true, false); } public ITask Pull(string remote, string branch) { var task = GitClient.Pull(remote, branch); - return HookupHandlers(task); + return HookupHandlers(task, true, true); } public ITask Push(string remote, string branch) { var task = GitClient.Push(remote, branch); - return HookupHandlers(task, false); + return HookupHandlers(task, true, false); } public ITask Revert(string changeset) { var task = GitClient.Revert(changeset); - return HookupHandlers(task); + return HookupHandlers(task, true, true); } public ITask RemoteAdd(string remote, string url) { var task = GitClient.RemoteAdd(remote, url); - task = HookupHandlers(task, false); + task = HookupHandlers(task, true, false); if (!platform.Environment.IsWindows) { task.Then(_ => { @@ -241,7 +241,7 @@ public ITask RemoteAdd(string remote, string url) public ITask RemoteRemove(string remote) { var task = GitClient.RemoteRemove(remote); - task = HookupHandlers(task, false); + task = HookupHandlers(task, true, false); if (!platform.Environment.IsWindows) { task.Then(_ => { @@ -254,25 +254,25 @@ public ITask RemoteRemove(string remote) public ITask RemoteChange(string remote, string url) { var task = GitClient.RemoteChange(remote, url); - return HookupHandlers(task, false); + return HookupHandlers(task, true, false); } public ITask SwitchBranch(string branch) { var task = GitClient.SwitchBranch(branch); - return HookupHandlers(task); + return HookupHandlers(task, true, true); } public ITask DeleteBranch(string branch, bool deleteUnmerged = false) { var task = GitClient.DeleteBranch(branch, deleteUnmerged); - return HookupHandlers(task, false); + return HookupHandlers(task, true, false); } public ITask CreateBranch(string branch, string baseBranch) { var task = GitClient.CreateBranch(branch, baseBranch); - return HookupHandlers(task, false); + return HookupHandlers(task, true, false); } public ITask> ListLocks(bool local) @@ -285,13 +285,13 @@ public ITask> ListLocks(bool local) public ITask LockFile(string file) { var task = GitClient.Lock(file); - return HookupHandlers(task, false); + return HookupHandlers(task, true, false); } public ITask UnlockFile(string file, bool force) { var task = GitClient.Unlock(file, force); - return HookupHandlers(task, false); + return HookupHandlers(task, true, false); } public void UpdateConfigData() @@ -328,16 +328,16 @@ private void UpdateHead() UpdateCurrentBranchAndRemote(head); } - private ITask HookupHandlers(ITask task, bool disableWatcher = true, bool toggleBusyFlag = true) + private ITask HookupHandlers(ITask task, bool isExclusive, bool filesystemChangesExpected) { return new ActionTask(CancellationToken.None, () => { - if (toggleBusyFlag) + if (isExclusive) { Logger.Trace("Starting Operation - Setting Busy Flag"); IsBusy = true; } - if (disableWatcher) + if (filesystemChangesExpected) { Logger.Trace("Starting Operation - Disable Watcher"); watcher.Stop(); @@ -345,13 +345,13 @@ private ITask HookupHandlers(ITask task, bool disableWatcher = true, bo }) .Then(task) .Finally((success, exception, result) => { - if (disableWatcher) + if (filesystemChangesExpected) { Logger.Trace("Ended Operation - Enable Watcher"); watcher.Start(); } - if (toggleBusyFlag) + if (isExclusive) { Logger.Trace("Ended Operation - Clearing Busy Flag"); IsBusy = false;