Merge remote-tracking branch 'remotes/origin/master' into enhancements/repository-watcher-refactor-rollup

StanleyGoldman · StanleyGoldman · commit 8c37affa5d0d · 2017-11-22T10:07:25.000-05:00
diff --git 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
@@ -168,64 +169,62 @@ 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));
+
+            return HookupHandlers(task, true, true);
         }
 
         public ITask CommitFiles(List<string> 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));
+
+            return HookupHandlers(task, true, true);
         }
 
         public ITask<List<GitLogEntry>> Log()
         {
             var task = GitClient.Log();
-            HookupHandlers(task);
-            return task;
+            return HookupHandlers(task, false, false);
         }
 
         public ITask<GitStatus> Status()
         {
             var task = GitClient.Status();
-            HookupHandlers(task);
-            return task;
+            return HookupHandlers(task, true, false);
         }
 
         public ITask Fetch(string remote)
         {
             var task = GitClient.Fetch(remote);
-            return HookupHandlers(task);
+            return HookupHandlers(task, true, false);
         }
 
         public ITask Pull(string remote, string branch)
         {
             var task = GitClient.Pull(remote, branch);
-            return HookupHandlers(task, true);
+            return HookupHandlers(task, true, true);
         }
 
         public ITask Push(string remote, string branch)
         {
             var task = GitClient.Push(remote, branch);
-            return HookupHandlers(task);
+            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);
-            HookupHandlers(task);
+            task = HookupHandlers(task, true, false);
             if (!platform.Environment.IsWindows)
             {
                 task.Then(_ => {
@@ -238,7 +237,7 @@ public ITask RemoteAdd(string remote, string url)
         public ITask RemoteRemove(string remote)
         {
             var task = GitClient.RemoteRemove(remote);
-            HookupHandlers(task);
+            task = HookupHandlers(task, true, false);
             if (!platform.Environment.IsWindows)
             {
                 task.Then(_ => {
@@ -251,44 +250,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, true, false);
         }
 
         public ITask SwitchBranch(string branch)
         {
             var task = GitClient.SwitchBranch(branch);
-            return HookupHandlers(task, true);
+            return HookupHandlers(task, true, true);
         }
 
         public ITask DeleteBranch(string branch, bool deleteUnmerged = false)
         {
             var task = GitClient.DeleteBranch(branch, deleteUnmerged);
-            return HookupHandlers(task);
+            return HookupHandlers(task, true, false);
         }
 
         public ITask CreateBranch(string branch, string baseBranch)
         {
             var task = GitClient.CreateBranch(branch, baseBranch);
-            return HookupHandlers(task);
+            return HookupHandlers(task, true, false);
         }
 
         public ITask<List<GitLock>> 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, true, false);
         }
 
         public ITask UnlockFile(string file, bool force)
         {
             var task = GitClient.Unlock(file, force);
-            return HookupHandlers(task);
+            return HookupHandlers(task, true, false);
         }
 
         private void SetupWatcher()
@@ -311,29 +310,42 @@ private void UpdateHead()
             UpdateCurrentBranchAndRemote(head);
         }
 
-        private ITask HookupHandlers(ITask task, bool disableWatcher = false)
+        private ITask<T> HookupHandlers<T>(ITask<T> task, bool isExclusive, bool filesystemChangesExpected)
         {
-            task.OnStart += t => {
-                Logger.Trace("Start " + task.Name);
-                IsBusy = true;
+            return new ActionTask(CancellationToken.None, () => {
+                    if (isExclusive)
+                    {
+                        Logger.Trace("Starting Operation - Setting Busy Flag");
+                        IsBusy = true;
+                    }
 
-                if (disableWatcher)
-                {
-                    watcher.Stop();
-                }
-            };
+                    if (filesystemChangesExpected)
+                    {
+                        Logger.Trace("Starting Operation - Disable Watcher");
+                        watcher.Stop();
+                    }
+                })
+                .Then(task)
+                .Finally((success, exception, result) => {
+                    if (filesystemChangesExpected)
+                    {
+                        Logger.Trace("Ended Operation - Enable Watcher");
+                        watcher.Start();
+                    }
 
-            task.OnEnd += t => {
-                if (disableWatcher)
-                {
-                    watcher.Start();
-                }
+                    if (isExclusive)
+                    {
+                        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)
diff --git 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>();
+            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()
         {

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`using System;`
`2`	`2`	`using System.Collections.Generic;`
`3`	`3`	`using System.Linq;`
	`4`	`+using System.Threading;`
`4`	`5`	`using System.Threading.Tasks;`
`5`	`6`
`6`	`7`	`namespace GitHub.Unity`
`@@ -168,64 +169,62 @@ public int WaitForEvents()`
`168`	`169`
`169`	`170`	`public ITask CommitAllFiles(string message, string body)`
`170`	`171`	`{`
`171`		`- var add = GitClient.AddAll();`
`172`		`- add.OnStart += t => IsBusy = true;`
`173`		`- return add`
`174`		`- .Then(GitClient.Commit(message, body))`
`175`		`- .Finally(() => IsBusy = false);`
	`172`	`+ var task = GitClient`
	`173`	`+ .AddAll()`
	`174`	`+ .Then(GitClient.Commit(message, body));`
	`175`	`+`
	`176`	`+ return HookupHandlers(task, true, true);`
`176`	`177`	`}`
`177`	`178`
`178`	`179`	`public ITask CommitFiles(List<string> files, string message, string body)`
`179`	`180`	`{`
`180`		`- var add = GitClient.Add(files);`
`181`		`- add.OnStart += t => IsBusy = true;`
`182`		`- return add`
`183`		`- .Then(GitClient.Commit(message, body))`
`184`		`- .Finally(() => IsBusy = false);`
	`181`	`+ var task = GitClient`
	`182`	`+ .Add(files)`
	`183`	`+ .Then(GitClient.Commit(message, body));`
	`184`	`+`
	`185`	`+ return HookupHandlers(task, true, true);`
`185`	`186`	`}`
`186`	`187`
`187`	`188`	`public ITask<List<GitLogEntry>> Log()`
`188`	`189`	`{`
`189`	`190`	`var task = GitClient.Log();`
`190`		`- HookupHandlers(task);`
`191`		`- return task;`
	`191`	`+ return HookupHandlers(task, false, false);`
`192`	`192`	`}`
`193`	`193`
`194`	`194`	`public ITask<GitStatus> Status()`
`195`	`195`	`{`
`196`	`196`	`var task = GitClient.Status();`
`197`		`- HookupHandlers(task);`
`198`		`- return task;`
	`197`	`+ return HookupHandlers(task, true, false);`
`199`	`198`	`}`
`200`	`199`
`201`	`200`	`public ITask Fetch(string remote)`
`202`	`201`	`{`
`203`	`202`	`var task = GitClient.Fetch(remote);`
`204`		`- return HookupHandlers(task);`
	`203`	`+ return HookupHandlers(task, true, false);`
`205`	`204`	`}`
`206`	`205`
`207`	`206`	`public ITask Pull(string remote, string branch)`
`208`	`207`	`{`
`209`	`208`	`var task = GitClient.Pull(remote, branch);`
`210`		`- return HookupHandlers(task, true);`
	`209`	`+ return HookupHandlers(task, true, true);`
`211`	`210`	`}`
`212`	`211`
`213`	`212`	`public ITask Push(string remote, string branch)`
`214`	`213`	`{`
`215`	`214`	`var task = GitClient.Push(remote, branch);`
`216`		`- return HookupHandlers(task);`
	`215`	`+ return HookupHandlers(task, true, false);`
`217`	`216`	`}`
`218`	`217`
`219`	`218`	`public ITask Revert(string changeset)`
`220`	`219`	`{`
`221`	`220`	`var task = GitClient.Revert(changeset);`
`222`		`- return HookupHandlers(task);`
	`221`	`+ return HookupHandlers(task, true, true);`
`223`	`222`	`}`
`224`	`223`
`225`	`224`	`public ITask RemoteAdd(string remote, string url)`
`226`	`225`	`{`
`227`	`226`	`var task = GitClient.RemoteAdd(remote, url);`
`228`		`- HookupHandlers(task);`
	`227`	`+ task = HookupHandlers(task, true, false);`
`229`	`228`	`if (!platform.Environment.IsWindows)`
`230`	`229`	`{`
`231`	`230`	`task.Then(_ => {`
`@@ -238,7 +237,7 @@ public ITask RemoteAdd(string remote, string url)`
`238`	`237`	`public ITask RemoteRemove(string remote)`
`239`	`238`	`{`
`240`	`239`	`var task = GitClient.RemoteRemove(remote);`
`241`		`- HookupHandlers(task);`
	`240`	`+ task = HookupHandlers(task, true, false);`
`242`	`241`	`if (!platform.Environment.IsWindows)`
`243`	`242`	`{`
`244`	`243`	`task.Then(_ => {`
`@@ -251,44 +250,44 @@ public ITask RemoteRemove(string remote)`
`251`	`250`	`public ITask RemoteChange(string remote, string url)`
`252`	`251`	`{`
`253`	`252`	`var task = GitClient.RemoteChange(remote, url);`
`254`		`- return HookupHandlers(task);`
	`253`	`+ return HookupHandlers(task, true, false);`
`255`	`254`	`}`
`256`	`255`
`257`	`256`	`public ITask SwitchBranch(string branch)`
`258`	`257`	`{`
`259`	`258`	`var task = GitClient.SwitchBranch(branch);`
`260`		`- return HookupHandlers(task, true);`
	`259`	`+ return HookupHandlers(task, true, true);`
`261`	`260`	`}`
`262`	`261`
`263`	`262`	`public ITask DeleteBranch(string branch, bool deleteUnmerged = false)`
`264`	`263`	`{`
`265`	`264`	`var task = GitClient.DeleteBranch(branch, deleteUnmerged);`
`266`		`- return HookupHandlers(task);`
	`265`	`+ return HookupHandlers(task, true, false);`
`267`	`266`	`}`
`268`	`267`
`269`	`268`	`public ITask CreateBranch(string branch, string baseBranch)`
`270`	`269`	`{`
`271`	`270`	`var task = GitClient.CreateBranch(branch, baseBranch);`
`272`		`- return HookupHandlers(task);`
	`271`	`+ return HookupHandlers(task, true, false);`
`273`	`272`	`}`
`274`	`273`
`275`	`274`	`public ITask<List<GitLock>> ListLocks(bool local)`
`276`	`275`	`{`
`277`	`276`	`var task = GitClient.ListLocks(local);`
`278`		`- HookupHandlers(task);`
	`277`	`+ HookupHandlers(task, false, false);`
`279`	`278`	`return task;`
`280`	`279`	`}`
`281`	`280`
`282`	`281`	`public ITask LockFile(string file)`
`283`	`282`	`{`
`284`	`283`	`var task = GitClient.Lock(file);`
`285`		`- return HookupHandlers(task);`
	`284`	`+ return HookupHandlers(task, true, false);`
`286`	`285`	`}`
`287`	`286`
`288`	`287`	`public ITask UnlockFile(string file, bool force)`
`289`	`288`	`{`
`290`	`289`	`var task = GitClient.Unlock(file, force);`
`291`		`- return HookupHandlers(task);`
	`290`	`+ return HookupHandlers(task, true, false);`
`292`	`291`	`}`
`293`	`292`
`294`	`293`	`private void SetupWatcher()`
`@@ -311,29 +310,42 @@ private void UpdateHead()`
`311`	`310`	`UpdateCurrentBranchAndRemote(head);`
`312`	`311`	`}`
`313`	`312`
`314`		`- private ITask HookupHandlers(ITask task, bool disableWatcher = false)`
	`313`	`+ private ITask<T> HookupHandlers<T>(ITask<T> task, bool isExclusive, bool filesystemChangesExpected)`
`315`	`314`	`{`
`316`		`- task.OnStart += t => {`
`317`		`- Logger.Trace("Start " + task.Name);`
`318`		`- IsBusy = true;`
	`315`	`+ return new ActionTask(CancellationToken.None, () => {`
	`316`	`+ if (isExclusive)`
	`317`	`+ {`
	`318`	`+ Logger.Trace("Starting Operation - Setting Busy Flag");`
	`319`	`+ IsBusy = true;`
	`320`	`+ }`
`319`	`321`
`320`		`- if (disableWatcher)`
`321`		`- {`
`322`		`- watcher.Stop();`
`323`		`- }`
`324`		`- };`
	`322`	`+ if (filesystemChangesExpected)`
	`323`	`+ {`
	`324`	`+ Logger.Trace("Starting Operation - Disable Watcher");`
	`325`	`+ watcher.Stop();`
	`326`	`+ }`
	`327`	`+ })`
	`328`	`+ .Then(task)`
	`329`	`+ .Finally((success, exception, result) => {`
	`330`	`+ if (filesystemChangesExpected)`
	`331`	`+ {`
	`332`	`+ Logger.Trace("Ended Operation - Enable Watcher");`
	`333`	`+ watcher.Start();`
	`334`	`+ }`
`325`	`335`
`326`		`- task.OnEnd += t => {`
`327`		`- if (disableWatcher)`
`328`		`- {`
`329`		`- watcher.Start();`
`330`		`- }`
	`336`	`+ if (isExclusive)`
	`337`	`+ {`
	`338`	`+ Logger.Trace("Ended Operation - Clearing Busy Flag");`
	`339`	`+ IsBusy = false;`
	`340`	`+ }`
`331`	`341`
`332`		`- IsBusy = false;`
	`342`	`+ if (success)`
	`343`	`+ {`
	`344`	`+ return result;`
	`345`	`+ }`
`333`	`346`
`334`		`- Logger.Trace("Finish " + task.Name);`
`335`		`- };`
`336`		`- return task;`
	`347`	`+ throw exception;`
	`348`	`+ });`
`337`	`349`	`}`
`338`	`350`
`339`	`351`	`private void Watcher_OnRemoteBranchDeleted(string remote, string name)`