Merge pull request #817 from github-for-unity/fixes/synccontext

Don't trample an existing SynchronizationContext

Author: StanleyGoldman

src/GitHub.Api/Application/ApiClient.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Threading.Tasks;
 using GitHub.Logging;
 using System.Runtime.Serialization;
 using System.Text;
@@ -33,127 +32,25 @@ public ApiClient(UriString hostUrl, IKeychain keychain, IProcessManager processM
             this.taskManager = taskManager;
             this.nodeJsExecutablePath = nodeJsExecutablePath;
             this.octorunScriptPath = octorunScriptPath;
-            loginManager = new LoginManager(keychain,
-                processManager: processManager,
-                taskManager: taskManager,
-                nodeJsExecutablePath: nodeJsExecutablePath,
-                octorunScript: octorunScriptPath);
+            loginManager = new LoginManager(keychain, processManager, taskManager, nodeJsExecutablePath, octorunScriptPath);
         }
 
         public ITask Logout(UriString host)
         {
             return loginManager.Logout(host);
         }
 
-        public async Task CreateRepository(string name, string description, bool isPrivate, Action<GitHubRepository, Exception> callback, string organization = null)
+        public void CreateRepository(string name, string description, bool isPrivate, Action<GitHubRepository, Exception> callback, string organization = null)
         {
             Guard.ArgumentNotNull(callback, "callback");
-            try
-            {
-                var repository = await CreateRepositoryInternal(name, organization, description, isPrivate);
-                callback(repository, null);
-            }
-            catch (Exception e)
-            {
-                callback(null, e);
-            }
-        }
-
-        public async Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null)
-        {
-            Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
-            await GetOrganizationInternal(onSuccess, onError);
-        }
 
-        public async Task GetCurrentUser(Action<GitHubUser> onSuccess, Action<Exception> onError = null)
-        {
-            Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
-            try
-            {
-                var user = await GetCurrentUser();
-                onSuccess(user);
-            }
-            catch (Exception e)
+            new FuncTask<GitHubRepository>(taskManager.Token, () =>
             {
-                onError?.Invoke(e);
-            }
-        }
-
-        public async Task Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result)
-        {
-            Guard.ArgumentNotNull(need2faCode, "need2faCode");
-            Guard.ArgumentNotNull(result, "result");
-
-            LoginResultData res = null;
-            try
-            {
-                res = await loginManager.Login(OriginalUrl, username, password);
-            }
-            catch (Exception ex)
-            {
-                logger.Warning(ex);
-                result(false, ex.Message);
-                return;
-            }
-
-            if (res.Code == LoginResultCodes.CodeRequired)
-            {
-                var resultCache = new LoginResult(res, result, need2faCode);
-                need2faCode(resultCache);
-            }
-            else
-            {
-                result(res.Code == LoginResultCodes.Success, res.Message);
-            }
-        }
-
-        public async Task ContinueLogin(LoginResult loginResult, string code)
-        {
-            LoginResultData result = null;
-            try
-            {
-                result = await loginManager.ContinueLogin(loginResult.Data, code);
-            }
-            catch (Exception ex)
-            {
-                loginResult.Callback(false, ex.Message);
-                return;
-            }
-            if (result.Code == LoginResultCodes.CodeFailed)
-            {
-                loginResult.TwoFACallback(new LoginResult(result, loginResult.Callback, loginResult.TwoFACallback));
-            }
-            loginResult.Callback(result.Code == LoginResultCodes.Success, result.Message);
-        }
-
-        private async Task<GitHubUser> GetCurrentUser()
-        {
-            //TODO: ONE_USER_LOGIN This assumes we only support one login
-            var keychainConnection = keychain.Connections.FirstOrDefault();
-            if (keychainConnection == null)
-                throw new KeychainEmptyException();
-
-            var keychainAdapter = await GetValidatedKeychainAdapter(keychainConnection);
-
-            // we can't trust that the system keychain has the username filled out correctly.
-            // if it doesn't, we need to grab the username from the server and check it
-            // unfortunately this means that things will be slower when the keychain doesn't have all the info
-            if (keychainConnection.User == null || keychainAdapter.Credential.Username != keychainConnection.Username)
-            {
-                keychainConnection.User = await GetValidatedGitHubUser(keychainConnection, keychainAdapter);
-            }
-            return keychainConnection.User;
-        }
-
-        private async Task<GitHubRepository> CreateRepositoryInternal(string repositoryName, string organization, string description, bool isPrivate)
-        {
-            try
-            {
-                var user = await GetCurrentUser();
+                var user = GetCurrentUser();
                 var keychainAdapter = keychain.Connect(OriginalUrl);
 
                 var command = new StringBuilder("publish -r \"");
-                command.Append(repositoryName);
+                command.Append(name);
                 command.Append("\"");
 
                 if (!string.IsNullOrEmpty(description))
@@ -176,10 +73,10 @@ private async Task<GitHubRepository> CreateRepositoryInternal(string repositoryN
                 }
 
                 var octorunTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath, octorunScriptPath, command.ToString(),
-                    user: user.Login, userToken: keychainAdapter.Credential.Token)
+                        user: user.Login, userToken: keychainAdapter.Credential.Token)
                     .Configure(processManager);
 
-                var ret = await octorunTask.StartAwait();
+                var ret = octorunTask.RunSynchronously();
                 if (ret.IsSuccess && ret.Output.Length == 2)
                 {
                     return new GitHubRepository
@@ -190,26 +87,33 @@ private async Task<GitHubRepository> CreateRepositoryInternal(string repositoryN
                 }
 
                 throw new ApiClientException(ret.GetApiErrorMessage() ?? "Publish failed");
-            }
-            catch (Exception ex)
+            })
+            .FinallyInUI((success, ex, repository) =>
             {
-                logger.Error(ex, "Error Creating Repository");
-                throw;
-            }
+                if (success)
+                    callback(repository, null);
+                else
+                {
+                    logger.Error(ex, "Error creating repository");
+                    callback(null, ex);
+                }
+            })
+            .Start();
         }
 
-        private async Task GetOrganizationInternal(Action<Organization[]> onSuccess, Action<Exception> onError = null)
+        public void GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null)
         {
-            try
+            Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
+            new FuncTask<Organization[]>(taskManager.Token, () =>
             {
-                var user = await GetCurrentUser();
+                var user = GetCurrentUser();
                 var keychainAdapter = keychain.Connect(OriginalUrl);
 
                 var octorunTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath, octorunScriptPath, "organizations",
                         user: user.Login, userToken: keychainAdapter.Credential.Token)
                     .Configure(processManager);
 
-                var ret = await octorunTask.StartAsAsync();
+                var ret = octorunTask.RunSynchronously();
                 if (ret.IsSuccess)
                 {
                     var organizations = new List<Organization>();
@@ -221,23 +125,109 @@ private async Task GetOrganizationInternal(Action<Organization[]> onSuccess, Act
                             Login = ret.Output[i + 1]
                         });
                     }
-
-                    onSuccess(organizations.ToArray());
-                    return;
+                    return organizations.ToArray();
                 }
 
                 throw new ApiClientException(ret.GetApiErrorMessage() ?? "Error getting organizations");
-            }
-            catch (Exception ex)
+            })
+            .FinallyInUI((success, ex, orgs) =>
+            {
+                if (success)
+                    onSuccess(orgs);
+                else
+                {
+                    logger.Error(ex, "Error Getting Organizations");
+                    onError?.Invoke(ex);
+                }
+            })
+            .Start();
+        }
+
+        public void GetCurrentUser(Action<GitHubUser> onSuccess, Action<Exception> onError = null)
+        {
+            Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
+            new FuncTask<GitHubUser>(taskManager.Token, GetCurrentUser)
+                .FinallyInUI((success, ex, user) =>
+                {
+                    if (success)
+                        onSuccess(user);
+                    else
+                        onError?.Invoke(ex);
+                })
+                .Start();
+        }
+
+        public void Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result)
+        {
+            Guard.ArgumentNotNull(need2faCode, "need2faCode");
+            Guard.ArgumentNotNull(result, "result");
+
+            new FuncTask<LoginResultData>(taskManager.Token,
+                () => loginManager.Login(OriginalUrl, username, password))
+                .FinallyInUI((success, ex, res) =>
+                {
+                    if (!success)
+                    {
+                        logger.Warning(ex);
+                        result(false, ex.Message);
+                        return;
+                    }
+
+                    if (res.Code == LoginResultCodes.CodeRequired)
+                    {
+                        var resultCache = new LoginResult(res, result, need2faCode);
+                        need2faCode(resultCache);
+                    }
+                    else
+                    {
+                        result(res.Code == LoginResultCodes.Success, res.Message);
+                    }
+                })
+                .Start();
+        }
+
+        public void ContinueLogin(LoginResult loginResult, string code)
+        {
+            new FuncTask<LoginResultData>(taskManager.Token,
+                () => loginManager.ContinueLogin(loginResult.Data, code))
+                .FinallyInUI((success, ex, result) =>
+                {
+                    if (!success)
+                    {
+                        loginResult.Callback(false, ex.Message);
+                        return;
+                    }
+                    if (result.Code == LoginResultCodes.CodeFailed)
+                    {
+                        loginResult.TwoFACallback(new LoginResult(result, loginResult.Callback, loginResult.TwoFACallback));
+                    }
+                    loginResult.Callback(result.Code == LoginResultCodes.Success, result.Message);
+                })
+                .Start();
+        }
+
+        private GitHubUser GetCurrentUser()
+        {
+            //TODO: ONE_USER_LOGIN This assumes we only support one login
+            var keychainConnection = keychain.Connections.FirstOrDefault();
+            if (keychainConnection == null)
+                throw new KeychainEmptyException();
+
+            var keychainAdapter = GetValidatedKeychainAdapter(keychainConnection);
+
+            // we can't trust that the system keychain has the username filled out correctly.
+            // if it doesn't, we need to grab the username from the server and check it
+            // unfortunately this means that things will be slower when the keychain doesn't have all the info
+            if (keychainConnection.User == null || keychainAdapter.Credential.Username != keychainConnection.Username)
             {
-                logger.Error(ex, "Error Getting Organizations");
-                onError?.Invoke(ex);
+                keychainConnection.User = GetValidatedGitHubUser(keychainConnection, keychainAdapter);
             }
+            return keychainConnection.User;
         }
 
-        private async Task<IKeychainAdapter> GetValidatedKeychainAdapter(Connection keychainConnection)
+        private IKeychainAdapter GetValidatedKeychainAdapter(Connection keychainConnection)
         {
-            var keychainAdapter = await keychain.Load(keychainConnection.Host);
+            var keychainAdapter = keychain.Load(keychainConnection.Host);
             if (keychainAdapter == null)
                 throw new KeychainEmptyException();
 
@@ -255,15 +245,15 @@ private async Task<IKeychainAdapter> GetValidatedKeychainAdapter(Connection keyc
             return keychainAdapter;
         }
 
-        private async Task<GitHubUser> GetValidatedGitHubUser(Connection keychainConnection, IKeychainAdapter keychainAdapter)
+        private GitHubUser GetValidatedGitHubUser(Connection keychainConnection, IKeychainAdapter keychainAdapter)
         {
             try
             {
                 var octorunTask = new OctorunTask(taskManager.Token, nodeJsExecutablePath, octorunScriptPath, "validate",
                         user: keychainConnection.Username, userToken: keychainAdapter.Credential.Token)
                     .Configure(processManager);
 
-                var ret = await octorunTask.StartAsAsync();
+                var ret = octorunTask.RunSynchronously();
                 if (ret.IsSuccess)
                 {
                     var login = ret.Output[1];

src/GitHub.Api/Application/ApplicationManagerBase.cs

@@ -28,10 +28,10 @@ public event Action<IProgress> OnProgress
 
         public ApplicationManagerBase(SynchronizationContext synchronizationContext, IEnvironment environment)
         {
+            UIScheduler = ThreadingHelper.GetUIScheduler(synchronizationContext);
+
             SynchronizationContext = synchronizationContext;
-            SynchronizationContext.SetSynchronizationContext(SynchronizationContext);
             ThreadingHelper.SetUIThread();
-            UIScheduler = TaskScheduler.FromCurrentSynchronizationContext();
             ThreadingHelper.MainThreadScheduler = UIScheduler;
 
             Environment = environment;

src/GitHub.Api/Application/IApiClient.cs

@@ -1,18 +1,17 @@
-using System.Threading.Tasks;
-using System;
+using System;
 
 namespace GitHub.Unity
 {
     interface IApiClient
     {
         HostAddress HostAddress { get; }
         UriString OriginalUrl { get; }
-        Task CreateRepository(string name, string description, bool isPrivate,
+        void CreateRepository(string name, string description, bool isPrivate,
             Action<GitHubRepository, Exception> callback, string organization = null);
-        Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null);
-        Task Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result);
-        Task ContinueLogin(LoginResult loginResult, string code);
+        void GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null);
+        void Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result);
+        void ContinueLogin(LoginResult loginResult, string code);
         ITask Logout(UriString host);
-        Task GetCurrentUser(Action<GitHubUser> onSuccess, Action<Exception> onError = null);
+        void GetCurrentUser(Action<GitHubUser> onSuccess, Action<Exception> onError = null);
     }
 }

src/GitHub.Api/Authentication/ICredentialManager.cs

@@ -13,10 +13,10 @@ public interface ICredential : IDisposable
 
     public interface ICredentialManager
     {
-        Task<ICredential> Load(UriString host);
-        Task Save(ICredential cred);
-        Task Delete(UriString host);
+        ICredential Load(UriString host);
+        void Save(ICredential cred);
+        void Delete(UriString host);
         bool HasCredentials();
         ICredential CachedCredentials { get; }
     }
-}
+}