Merge pull request #378 from github-for-unity/stanley/branches-view-fixes

Edits to the branches-view branch

Author: shana

GitHub.Unity.sln.DotSettings

@@ -342,6 +342,7 @@
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EAlwaysTreatStructAsNotReorderableMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateThisQualifierSettings/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EUnitTestFramework_002ESettings_002EMigrations_002ERemoveBuildPolicyAlwaysMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/UnitTesting/ShadowCopy/@EntryValue">False</s:Boolean>
 	<s:String x:Key="/Default/Housekeeping/UnitTestingMru/UnitTestSessionDefault/PlatformType/@EntryValue">x64</s:String>
 	</wpf:ResourceDictionary>

README.md

@@ -47,8 +47,8 @@ The GitHub for Unity extension brings [Git](https://git-scm.com/) and GitHub int
 
 ### Requirements
 
-- Unity 5.4-5.6
-   - We've only tested the extension so far on Unity 5.4 to 5.6. There's currently an blocker issue opened for 5.3 support, so we know it doesn't run there. There are some issues for 2017.x, so it may or may not run well on that version. Personal edition is fine.
+- Unity 5.4-2017.1
+   - We've only tested the extension so far on Unity 5.4 to 2017.1. There's currently an blocker issue opened for 5.3 support, so we know it doesn't run there. There are some issues for 2017.2, so it may or may not run well on that version. Personal edition is fine.
 - Git and Git LFS 2.x
 
 #### Git on macOS

common/properties.props

@@ -7,6 +7,7 @@
 
     <UnityDir Condition="$(UnityDir) == '' and Exists('$(SolutionDir)\script\lib\UnityEditor.dll')">$(SolutionDir)\script\lib\</UnityDir>
     <UnityDir Condition="$(UnityDir) == '' and Exists('$(SolutionDir)\lib\UnityEditor.dll')">$(SolutionDir)\lib\</UnityDir>
+    <UnityDir Condition="$(UnityDir) == '' and Exists('C:\Program Files\Unity\Editor\Data\Managed\UnityEditor.dll')">C:\Program Files\Unity\Editor\Data\Managed\</UnityDir>
     <UnityDir Condition="$(UnityDir) == '' and Exists('C:\Program Files (x86)\Unity\Editor\Data\Managed\UnityEditor.dll')">C:\Program Files (x86)\Unity\Editor\Data\Managed\</UnityDir>
     <UnityDir Condition="$(UnityDir) == '' and Exists('\Applications\Unity\Unity.app\Contents\Managed\UnityEditor.dll')">\Applications\Unity\Unity.app\Contents\Managed\</UnityDir>
     <BuildConfig Condition=" '$(BuildConfig)' == '' ">Debug</BuildConfig>

docs/contributing/how-to-build.md

@@ -9,14 +9,14 @@ This repository is LFS-enabled. To clone it, you should use a git client that su
 - Visual Studio 2015+ or Mono 4.x + bash shell (git bash).
   - Mono 5.x will not work
 - `UnityEngine.dll` and `UnityEditor.dll`.
-  - If you've installed Unity in the default location of `C:\Program Files\Unity` or `C:\Program Files (x86)\Unity`, the build will be able to reference these DLLs automatically. Otherwise, you'll need to copy these DLLs from your Unity installation into the `lib` directory in order for the build to work
+  - If you've installed Unity in the default location of `C:\Program Files\Unity` or `C:\Program Files (x86)\Unity`, the build will be able to reference these DLLs automatically. Otherwise, you'll need to copy these DLLs from `[Unity installation path]\Unity\Editor\Data\Managed` into the `lib` directory in order for the build to work
 
 ### MacOS
 
 - Mono 4.x required.
   - Mono 5.x will not work
 - `UnityEngine.dll` and `UnityEditor.dll`.
-  - If you've installed Unity in the default location of `/Applications/Unity`, the build will be able to reference these DLLs automatically. Otherwise, you'll need to copy these DLLs from your Unity installation into the `lib` directory in order for the build to work
+  - If you've installed Unity in the default location of `/Applications/Unity`, the build will be able to reference these DLLs automatically. Otherwise, you'll need to copy these DLLs from `[Unity installation path]/Unity.app/Contents/Managed` into the `lib` directory in order for the build to work
 
 ## How to Build
 
@@ -40,11 +40,11 @@ To be able to authenticate in GitHub for Unity, you'll need to:
 - [Register a new developer application](https://github.com/settings/developers) in your profile.
 - Copy [common/ApplicationInfo_Local.cs-example](../../common/ApplicationInfo_Local.cs-example) to `common/ApplicationInfo_Local.cs` and fill out the clientId/clientSecret fields for your application.
 
-The build needs to reference `UnityEngine.dll` and `UnityEditor.dll`. These DLLs are included with Unity. If you've installed Unity in the default location, the build will be able to find them automatically. If not, copy these DLLs from your Unity installation into the `lib` directory in order for the build to work.
+The build needs to reference `UnityEngine.dll` and `UnityEditor.dll`. These DLLs are included with Unity. If you've installed Unity in the default location, the build will be able to find them automatically. If not, copy these DLLs from `[your Unity installation path]\Unity\Editor\Data\Managed` into the `lib` directory in order for the build to work.
 
 ### Visual Studio
 
-To build with Visual Studio 2015 open the solution file `GitHub.Unity.sln`. Select `Build Solution` in the `Build` menu.
+To build with Visual Studio 2015+, open the solution file `GitHub.Unity.sln`. Select `Build Solution` in the `Build` menu.
 
 ### Mono and Bash (windows and mac)
 

src/GitHub.Api/Application/ApiClient.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Threading;
 using System.Threading.Tasks;
 using Octokit;
 
@@ -20,20 +19,13 @@ public static IApiClient Create(UriString repositoryUrl, IKeychain keychain)
                 new GitHubClient(AppConfiguration.ProductHeader, credentialStore, hostAddress.ApiUri));
         }
 
-        private static readonly Unity.ILogging logger = Unity.Logging.GetLogger<ApiClient>();
+        private static readonly ILogging logger = Logging.GetLogger<ApiClient>();
         public HostAddress HostAddress { get; }
         public UriString OriginalUrl { get; }
 
         private readonly IKeychain keychain;
         private readonly IGitHubClient githubClient;
         private readonly ILoginManager loginManager;
-        private static readonly SemaphoreSlim sem = new SemaphoreSlim(1);
-
-        IList<Organization> organizationsCache;
-        Octokit.User userCache;
-
-        string owner;
-        bool? isEnterprise;
 
         public ApiClient(UriString hostUrl, IKeychain keychain, IGitHubClient githubClient)
         {
@@ -58,7 +50,7 @@ private async Task LogoutInternal(UriString host)
             await loginManager.Logout(host);
         }
 
-        public async Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null)
+        public async Task CreateRepository(NewRepository newRepository, Action<GitHubRepository, Exception> callback, string organization = null)
         {
             Guard.ArgumentNotNull(callback, "callback");
             try
@@ -72,21 +64,27 @@ public async Task CreateRepository(NewRepository newRepository, Action<Octokit.R
             }
         }
 
-        public async Task GetOrganizations(Action<IList<Organization>> callback)
+        public async Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null)
         {
-            Guard.ArgumentNotNull(callback, "callback");
-            var organizations = await GetOrganizationInternal();
-            callback(organizations);
+            Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
+            await GetOrganizationInternal(onSuccess, onError);
         }
 
-        public async Task LoadKeychain(Action<bool> callback)
+        public async Task ValidateCurrentUser(Action onSuccess, Action<Exception> onError = null)
         {
-            Guard.ArgumentNotNull(callback, "callback");
-            var hasLoadedKeys = await LoadKeychainInternal();
-            callback(hasLoadedKeys);
+            Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
+            try
+            {
+                await ValidateCurrentUserInternal();
+                onSuccess();
+            }
+            catch (Exception e)
+            {
+                onError?.Invoke(e);
+            }
         }
 
-        public async Task GetCurrentUser(Action<Octokit.User> callback)
+        public async Task GetCurrentUser(Action<GitHubUser> callback)
         {
             Guard.ArgumentNotNull(callback, "callback");
             var user = await GetCurrentUserInternal();
@@ -189,29 +187,27 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe
             return result.Code == LoginResultCodes.Success;
         }
 
-        private async Task<Octokit.Repository> CreateRepositoryInternal(NewRepository newRepository, string organization)
+        private async Task<GitHubRepository> CreateRepositoryInternal(NewRepository newRepository, string organization)
         {
             try
             {
                 logger.Trace("Creating repository");
 
-                if (!await LoadKeychainInternal())
-                {
-                    throw new InvalidOperationException("The keychain did not load");
-                }
+                await ValidateKeychain();
+                await ValidateCurrentUserInternal();
 
-                Octokit.Repository repository;
+                GitHubRepository repository;
                 if (!string.IsNullOrEmpty(organization))
                 {
                     logger.Trace("Creating repository for organization");
 
-                    repository = await githubClient.Repository.Create(organization, newRepository);
+                    repository = (await githubClient.Repository.Create(organization, newRepository)).ToGitHubRepository();
                 }
                 else
                 {
                     logger.Trace("Creating repository for user");
 
-                    repository = await githubClient.Repository.Create(newRepository);
+                    repository = (await githubClient.Repository.Create(newRepository)).ToGitHubRepository();
                 }
 
                 logger.Trace("Created Repository");
@@ -224,66 +220,78 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe
             }
         }
 
-        private async Task<IList<Organization>> GetOrganizationInternal()
+        private async Task GetOrganizationInternal(Action<Organization[]> onSuccess, Action<Exception> onError = null)
         {
             try
             {
                 logger.Trace("Getting Organizations");
 
-                if (!await LoadKeychainInternal())
-                {
-                    return new List<Organization>();
-                }
+                await ValidateKeychain();
+                await ValidateCurrentUserInternal();
 
                 var organizations = await githubClient.Organization.GetAllForCurrent();
 
                 logger.Trace("Obtained {0} Organizations", organizations?.Count.ToString() ?? "NULL");
 
                 if (organizations != null)
                 {
-                    organizationsCache = organizations.ToArray();
+                    var array = organizations.Select(organization => new Organization() {
+                        Name = organization.Name,
+                        Login = organization.Login
+                    }).ToArray();
+                    onSuccess(array);
                 }
             }
             catch(Exception ex)
             {
                 logger.Error(ex, "Error Getting Organizations");
-                throw;
+                onError?.Invoke(ex);
             }
-
-            return organizationsCache;
         }
 
-        private async Task<Octokit.User> GetCurrentUserInternal()
+        private async Task<GitHubUser> GetCurrentUserInternal()
         {
             try
             {
-                logger.Trace("Getting Organizations");
-
-                if (!await LoadKeychainInternal())
-                {
-                    return null;
-                }
+                logger.Trace("Getting Current User");
+                await ValidateKeychain();
 
-                userCache = await githubClient.User.Current();
+                return (await githubClient.User.Current()).ToGitHubUser();
             }
-            catch(Exception ex)
+            catch (KeychainEmptyException)
+            {
+                logger.Warning("Keychain is empty");
+                throw;
+            }
+            catch (Exception ex)
             {
                 logger.Error(ex, "Error Getting Current User");
                 throw;
             }
+        }
+
+        private async Task ValidateCurrentUserInternal()
+        {
+            logger.Trace("Validating User");
 
-            return userCache;
+            var apiUser = await GetCurrentUserInternal();
+            var apiUsername = apiUser.Login;
+
+            var cachedUsername = keychain.Connections.First().Username;
+
+            if (apiUsername != cachedUsername)
+            {
+                throw new TokenUsernameMismatchException(cachedUsername, apiUsername);
+            }
         }
 
         private async Task<bool> LoadKeychainInternal()
         {
-            logger.Trace("LoadKeychainInternal");
-
             if (keychain.HasKeys)
             {
                 if (!keychain.NeedsLoad)
                 {
-                    logger.Trace("LoadKeychainInternal: Has keys does not need load");
+                    logger.Trace("LoadKeychainInternal: Previously Loaded");
                     return true;
                 }
 
@@ -293,6 +301,8 @@ private async Task<bool> LoadKeychainInternal()
                 var uriString = keychain.Connections.First().Host;
                 var keychainAdapter = await keychain.Load(uriString);
 
+                logger.Trace("LoadKeychainInternal: Loaded");
+
                 return keychainAdapter.OctokitCredentials != Credentials.Anonymous;
             }
 
@@ -301,23 +311,54 @@ private async Task<bool> LoadKeychainInternal()
             return false;
         }
 
-        public async Task<bool> ValidateCredentials()
+        private async Task ValidateKeychain()
         {
-            try
+            if (!await LoadKeychainInternal())
             {
-                var store = keychain.Connect(OriginalUrl);
-
-                if (store.OctokitCredentials != Credentials.Anonymous)
-                {
-                    var credential = store.Credential;
-                    await githubClient.Authorization.CheckApplicationAuthentication(ApplicationInfo.ClientId, credential.Token);
-                }
+                throw new KeychainEmptyException();
             }
-            catch
-            {
-                return false;
-            }
-            return true;
         }
     }
+
+    class GitHubUser
+    {
+        public string Name { get; set; }
+        public string Login { get; set; }
+    }
+
+    class GitHubRepository
+    {
+        public string Name { get; set; }
+        public string CloneUrl { get; set; }
+    }
+
+    class ApiClientException : Exception
+    {
+        public ApiClientException()
+        { }
+
+        public ApiClientException(string message) : base(message)
+        { }
+
+        public ApiClientException(string message, Exception innerException) : base(message, innerException)
+        { }
+    }
+
+    class TokenUsernameMismatchException : ApiClientException
+    {
+        public string CachedUsername { get; }
+        public string CurrentUsername { get; }
+
+        public TokenUsernameMismatchException(string cachedUsername, string currentUsername)
+        {
+            CachedUsername = cachedUsername;
+            CurrentUsername = currentUsername;
+        }
+    }
+
+    class KeychainEmptyException : ApiClientException
+    {
+        public KeychainEmptyException()
+        { }
+    }
 }