Merge branch 'master' into fixes/settings-view-code-cleanup

Author: StanleyGoldman

.github/ISSUE_TEMPLATE.md

@@ -9,7 +9,7 @@ Have you read GitHub for Unity's Code of Conduct? By filing an Issue, you are ex
 - Be sure to run with tracing enabled to capture runtime details in the log file
 - Include the log file in the PR.
   - On Windows, the extension log file is at `%LOCALAPPDATA%\GitHubUnity\github-unity.log`
-  - On macOS, the extension log file is at `~/.local/share/GitHubUnity/github-unity.log`
+  - On macOS, the extension log file is at `~/Library/Application Support/GitHubUnity/github-unity.log`
 
 ### Description
 

common/SolutionInfo.cs

@@ -31,6 +31,6 @@
 namespace System
 {
     internal static class AssemblyVersionInformation {
-        internal const string Version = "0.16.0.0";
+        internal const string Version = "0.19.0.0";
     }
 }

src/GitHub.Api/Application/ApiClient.cs

@@ -29,7 +29,6 @@ public static IApiClient Create(UriString repositoryUrl, IKeychain keychain)
         private readonly ILoginManager loginManager;
         private static readonly SemaphoreSlim sem = new SemaphoreSlim(1);
 
-        Octokit.Repository repositoryCache = new Octokit.Repository();
         IList<Organization> organizationsCache;
         Octokit.User userCache;
 
@@ -49,13 +48,6 @@ public ApiClient(UriString hostUrl, IKeychain keychain, IGitHubClient githubClie
             loginManager = new LoginManager(keychain, ApplicationInfo.ClientId, ApplicationInfo.ClientSecret);
         }
 
-        public async Task GetRepository(Action<Octokit.Repository> callback)
-        {
-            Guard.ArgumentNotNull(callback, "callback");
-            var repo = await GetRepositoryInternal();
-            callback(repo);
-        }
-
         public async Task Logout(UriString host)
         {
             await LogoutInternal(host);
@@ -69,7 +61,15 @@ private async Task LogoutInternal(UriString host)
         public async Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null)
         {
             Guard.ArgumentNotNull(callback, "callback");
-            await CreateRepositoryInternal(newRepository, callback, organization);
+            try
+            {
+                var repository = await CreateRepositoryInternal(newRepository, organization);
+                callback(repository, null);
+            }
+            catch (Exception e)
+            {
+                callback(null, e);
+            }
         }
 
         public async Task GetOrganizations(Action<IList<Organization>> callback)
@@ -79,6 +79,13 @@ public async Task GetOrganizations(Action<IList<Organization>> callback)
             callback(organizations);
         }
 
+        public async Task LoadKeychain(Action<bool> callback)
+        {
+            Guard.ArgumentNotNull(callback, "callback");
+            var hasLoadedKeys = await LoadKeychainInternal();
+            callback(hasLoadedKeys);
+        }
+
         public async Task GetCurrentUser(Action<Octokit.User> callback)
         {
             Guard.ArgumentNotNull(callback, "callback");
@@ -182,65 +189,38 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe
             return result.Code == LoginResultCodes.Success;
         }
 
-        private async Task<Octokit.Repository> GetRepositoryInternal()
+        private async Task<Octokit.Repository> CreateRepositoryInternal(NewRepository newRepository, string organization)
         {
             try
             {
-                if (owner == null)
+                logger.Trace("Creating repository");
+
+                if (!await LoadKeychainInternal())
                 {
-                    var ownerLogin = OriginalUrl.Owner;
-                    var repositoryName = OriginalUrl.RepositoryName;
-
-                    if (ownerLogin != null && repositoryName != null)
-                    {
-                        var repo = await githubClient.Repository.Get(ownerLogin, repositoryName);
-                        if (repo != null)
-                        {
-                            repositoryCache = repo;
-                        }
-                        owner = ownerLogin;
-                    }
+                    throw new InvalidOperationException("The keychain did not load");
                 }
-            }
-            // it'll throw if it's private or an enterprise instance requiring authentication
-            catch (ApiException apiex)
-            {
-                if (!HostAddress.IsGitHubDotCom(OriginalUrl.ToRepositoryUri()))
-                    isEnterprise = apiex.IsGitHubApiException();
-            }
-            catch {}
-            finally
-            {
-                sem.Release();
-            }
-
-            return repositoryCache;
-        }
-
-        private async Task CreateRepositoryInternal(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization)
-        {
-            try
-            {
-                logger.Trace("Creating Repository");
 
                 Octokit.Repository repository;
-                if (organization != null)
+                if (!string.IsNullOrEmpty(organization))
                 {
+                    logger.Trace("Creating repository for organization");
+
                     repository = await githubClient.Repository.Create(organization, newRepository);
                 }
                 else
                 {
+                    logger.Trace("Creating repository for user");
+
                     repository = await githubClient.Repository.Create(newRepository);
                 }
 
                 logger.Trace("Created Repository");
-
-                callback(repository, null);
+                return repository;
             }
             catch (Exception ex)
             {
                 logger.Error(ex, "Error Creating Repository");
-                callback(null, ex);
+                throw;
             }
         }
 
@@ -250,6 +230,11 @@ private async Task<IList<Organization>> GetOrganizationInternal()
             {
                 logger.Trace("Getting Organizations");
 
+                if (!await LoadKeychainInternal())
+                {
+                    return new List<Organization>();
+                }
+
                 var organizations = await githubClient.Organization.GetAllForCurrent();
 
                 logger.Trace("Obtained {0} Organizations", organizations?.Count.ToString() ?? "NULL");
@@ -274,6 +259,11 @@ private async Task<IList<Organization>> GetOrganizationInternal()
             {
                 logger.Trace("Getting Organizations");
 
+                if (!await LoadKeychainInternal())
+                {
+                    return null;
+                }
+
                 userCache = await githubClient.User.Current();
             }
             catch(Exception ex)
@@ -285,6 +275,32 @@ private async Task<IList<Organization>> GetOrganizationInternal()
             return userCache;
         }
 
+        private async Task<bool> LoadKeychainInternal()
+        {
+            logger.Trace("LoadKeychainInternal");
+
+            if (keychain.HasKeys)
+            {
+                if (!keychain.NeedsLoad)
+                {
+                    logger.Trace("LoadKeychainInternal: Has keys does not need load");
+                    return true;
+                }
+
+                logger.Trace("LoadKeychainInternal: Loading");
+
+                //TODO: ONE_USER_LOGIN This assumes only ever one user can login
+                var uriString = keychain.Connections.First().Host;
+                var keychainAdapter = await keychain.Load(uriString);
+
+                return keychainAdapter.OctokitCredentials != Credentials.Anonymous;
+            }
+
+            logger.Trace("LoadKeychainInternal: No keys to load");
+
+            return false;
+        }
+
         public async Task<bool> ValidateCredentials()
         {
             try

src/GitHub.Api/Application/ApplicationManagerBase.cs

@@ -39,20 +39,24 @@ protected void Initialize()
             Logging.TracingEnabled = UserSettings.Get(Constants.TraceLoggingKey, false);
             ProcessManager = new ProcessManager(Environment, Platform.GitEnvironment, CancellationToken);
             Platform.Initialize(ProcessManager, TaskManager);
-            if (Environment.GitExecutablePath != null)
-            {
-                GitClient = new GitClient(Environment, ProcessManager, Platform.CredentialManager, TaskManager);
-            }
+            GitClient = new GitClient(Environment, ProcessManager, Platform.CredentialManager, TaskManager);
             SetupMetrics();
         }
 
-        public virtual async Task Run(bool firstRun)
+        public void Run(bool firstRun)
+        {
+            new ActionTask(SetupGit())
+                .Then(RestartRepository)
+                .ThenInUI(InitializeUI)
+                .Start();
+        }
+
+        private async Task SetupGit()
         {
             Logger.Trace("Run - CurrentDirectory {0}", NPath.CurrentDirectory);
 
             if (Environment.GitExecutablePath == null)
             {
-                GitClient = new GitClient(Environment, ProcessManager, Platform.CredentialManager, TaskManager);
                 Environment.GitExecutablePath = await DetermineGitExecutablePath();
 
                 Logger.Trace("Environment.GitExecutablePath \"{0}\" Exists:{1}", Environment.GitExecutablePath, Environment.GitExecutablePath.FileExists());
@@ -67,10 +71,6 @@ public virtual async Task Run(bool firstRun)
                 }
             }
 
-            RestartRepository();
-            InitializeUI();
-
-            new ActionTask(new Task(() => LoadKeychain().Start())).Start();
         }
 
         public ITask InitializeRepository()
@@ -79,7 +79,10 @@ public ITask InitializeRepository()
 
             var targetPath = NPath.CurrentDirectory;
 
-            var unityYamlMergeExec = Environment.UnityApplication.Parent.Combine("Tools", "UnityYAMLMerge");
+            var unityYamlMergeExec = Environment.IsWindows
+                ? Environment.UnityApplication.Parent.Combine("Data", "Tools", "UnityYAMLMerge.exe")
+                : Environment.UnityApplication.Combine("Contents", "Tools", "UnityYAMLMerge");
+
             var yamlMergeCommand = Environment.IsWindows
                 ? $@"'{unityYamlMergeExec}' merge -p ""$BASE"" ""$REMOTE"" ""$LOCAL"" ""$MERGED"""
                 : $@"'{unityYamlMergeExec}' merge -p '$BASE' '$REMOTE' '$LOCAL' '$MERGED'";
@@ -104,41 +107,27 @@ public ITask InitializeRepository()
                 }))
                 .Then(GitClient.Add(filesForInitialCommit))
                 .Then(GitClient.Commit("Initial commit", null))
-                .Then(RestartRepository)
+                .Then(_ =>
+                {
+                    Environment.InitializeRepository();
+                    RestartRepository();
+                })
                 .ThenInUI(InitializeUI);
             return task;
         }
 
         public void RestartRepository()
         {
-            Environment.InitializeRepository();
             if (Environment.RepositoryPath != null)
             {
                 repositoryManager = Unity.RepositoryManager.CreateInstance(Platform, TaskManager, UsageTracker, GitClient, Environment.RepositoryPath);
-                Environment.Repository = new Repository(GitClient, repositoryManager, Environment.RepositoryPath);
                 repositoryManager.Initialize();
-                Environment.Repository.Initialize();
+                Environment.Repository.Initialize(repositoryManager);
                 repositoryManager.Start();
                 Logger.Trace($"Got a repository? {Environment.Repository}");
             }
         }
 
-        private async Task LoadKeychain()
-        {
-            Logger.Trace("Loading Keychain");
-
-            var firstConnection = Platform.Keychain.Hosts.FirstOrDefault();
-            if (firstConnection == null)
-            {
-                Logger.Trace("No Host Found");
-            }
-            else
-            {
-                Logger.Trace("Loading Connection to Host:\"{0}\"", firstConnection);
-                await Platform.Keychain.Load(firstConnection).SafeAwait();
-            }
-        }
-
         private async Task<NPath> DetermineGitExecutablePath(ProgressReport progress = null)
         {
             var gitExecutablePath = SystemSettings.Get(Constants.GitInstallPathKey)?.ToNPath();

src/GitHub.Api/Application/IApiClient.cs

@@ -9,7 +9,6 @@ interface IApiClient
     {
         HostAddress HostAddress { get; }
         UriString OriginalUrl { get; }
-        Task GetRepository(Action<Octokit.Repository> callback);
         Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null);
         Task GetOrganizations(Action<IList<Organization>> callback);
         Task Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result);
@@ -18,5 +17,6 @@ interface IApiClient
         Task<bool> ValidateCredentials();
         Task Logout(UriString host);
         Task GetCurrentUser(Action<Octokit.User> callback);
+        Task LoadKeychain(Action<bool> callback);
     }
 }

src/GitHub.Api/Application/IApplicationManager.cs

@@ -17,6 +17,8 @@ interface IApplicationManager : IDisposable
         ITaskManager TaskManager { get; }
         IGitClient GitClient { get; }
         IUsageTracker UsageTracker { get; }
+
+        void Run(bool firstRun);
         void RestartRepository();
         ITask InitializeRepository();
     }

src/GitHub.Api/Authentication/IKeychain.cs

@@ -15,6 +15,7 @@ interface IKeychain
         Connection[] Connections { get; }
         IList<UriString> Hosts { get; }
         bool HasKeys { get; }
+        bool NeedsLoad { get; }
         void SetToken(UriString host, string token);
     }
 }

src/GitHub.Api/Authentication/Keychain.cs

@@ -230,5 +230,7 @@ public void UpdateToken(UriString host, string token)
         public IList<UriString> Hosts => connectionCache.Keys.ToArray();
 
         public bool HasKeys => connectionCache.Any();
+
+        public bool NeedsLoad => HasKeys && FindOrCreateAdapter(connectionCache.First().Value.Host).OctokitCredentials == Credentials.Anonymous;
     }
 }

src/GitHub.Api/Git/GitClient.cs

@@ -134,13 +134,18 @@ private Task<NPath> LookForPortableGit()
 
         private async Task<NPath> LookForSystemGit()
         {
-            if (environment.IsMac)
+            NPath path = null;
+            if (!environment.IsWindows)
             {
-                var path = "/usr/local/bin/git".ToNPath();
-                if (path.FileExists())
-                    return path;
+                var p = new NPath("/usr/local/bin/git");
+                if (p.FileExists())
+                    path = p;
             }
-            return await new FindExecTask("git", taskManager.Token).StartAwait();
+
+            if (path == null)
+                path = await new FindExecTask("git", taskManager.Token).StartAwait();
+
+            return path;
         }
 
         public bool ValidateGitInstall(NPath path)