Merge pull request #386 from github-for-unity/fixes/api-client-octokit-objects

shana · web-flow · commit cafb7e7555b2 · 2017-11-01T20:26:54.000-07:00
Preventing the exposure of Octokit objects to the codebase
diff --git a/src/GitHub.Api/Application/ApiClient.cs b/src/GitHub.Api/Application/ApiClient.cs
@@ -50,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
@@ -64,7 +64,7 @@ public async Task CreateRepository(NewRepository newRepository, Action<Octokit.R
             }
         }
 
-        public async Task GetOrganizations(Action<IList<Organization>> onSuccess, Action<Exception> onError = null)
+        public async Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null)
         {
             Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));
             await GetOrganizationInternal(onSuccess, onError);
@@ -84,7 +84,7 @@ public async Task ValidateCurrentUser(Action onSuccess, Action<Exception> onErro
             }
         }
 
-        public async Task GetCurrentUser(Action<Octokit.User> callback)
+        public async Task GetCurrentUser(Action<GitHubUser> callback)
         {
             Guard.ArgumentNotNull(callback, "callback");
             var user = await GetCurrentUserInternal();
@@ -187,7 +187,7 @@ 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
             {
@@ -196,18 +196,18 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe
                 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");
@@ -220,7 +220,7 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe
             }
         }
 
-        private async Task GetOrganizationInternal(Action<IList<Organization>> onSuccess, Action<Exception> onError = null)
+        private async Task GetOrganizationInternal(Action<Organization[]> onSuccess, Action<Exception> onError = null)
         {
             try
             {
@@ -235,7 +235,11 @@ private async Task GetOrganizationInternal(Action<IList<Organization>> onSuccess
 
                 if (organizations != null)
                 {
-                    onSuccess(organizations.ToArray());
+                    var array = organizations.Select(organization => new Organization() {
+                        Name = organization.Name,
+                        Login = organization.Login
+                    }).ToArray();
+                    onSuccess(array);
                 }
             }
             catch(Exception ex)
@@ -245,14 +249,14 @@ private async Task GetOrganizationInternal(Action<IList<Organization>> onSuccess
             }
         }
 
-        private async Task<Octokit.User> GetCurrentUserInternal()
+        private async Task<GitHubUser> GetCurrentUserInternal()
         {
             try
             {
                 logger.Trace("Getting Current User");
                 await ValidateKeychain();
 
-                return await githubClient.User.Current();
+                return (await githubClient.User.Current()).ToGitHubUser();
             }
             catch (Exception ex)
             {
@@ -311,6 +315,18 @@ private async Task ValidateKeychain()
         }
     }
 
+    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()
diff --git a/src/GitHub.Api/Application/IApiClient.cs b/src/GitHub.Api/Application/IApiClient.cs
@@ -9,13 +9,13 @@ interface IApiClient
     {
         HostAddress HostAddress { get; }
         UriString OriginalUrl { get; }
-        Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null);
-        Task GetOrganizations(Action<IList<Organization>> onSuccess, Action<Exception> onError = null);
+        Task CreateRepository(NewRepository newRepository, 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);
         Task<bool> LoginAsync(string username, string password, Func<LoginResult, string> need2faCode);
         Task Logout(UriString host);
-        Task GetCurrentUser(Action<Octokit.User> callback);
+        Task GetCurrentUser(Action<GitHubUser> callback);
         Task ValidateCurrentUser(Action onSuccess, Action<Exception> onError = null);
     }
 }
diff --git a/src/GitHub.Api/Application/OctokitExtensions.cs b/src/GitHub.Api/Application/OctokitExtensions.cs
@@ -0,0 +1,21 @@
+namespace GitHub.Unity
+{
+    static class OctokitExtensions
+    {
+        public static GitHubUser ToGitHubUser(this Octokit.User user)
+        {
+            return new GitHubUser() {
+                Name = user.Name,
+                Login = user.Login,
+            };
+        }
+
+        public static GitHubRepository ToGitHubRepository(this Octokit.Repository repository)
+        {
+            return new GitHubRepository {
+                Name = repository.Name,
+                CloneUrl = repository.CloneUrl
+            };
+        }
+    }
+}
diff --git a/src/GitHub.Api/Application/Organization.cs b/src/GitHub.Api/Application/Organization.cs
@@ -0,0 +1,8 @@
+﻿namespace GitHub.Unity
+{
+    class Organization
+    {
+        public string Name { get; set; }
+        public string Login { get; set; }
+    }
+}
diff --git a/src/GitHub.Api/GitHub.Api.csproj b/src/GitHub.Api/GitHub.Api.csproj
@@ -99,6 +99,8 @@
     <Compile Include="Application\ApplicationInfo.cs" />
     <Compile Include="Application\LoginResult.cs" />
     <Compile Include="Application\AppConfiguration.cs" />
+    <Compile Include="Application\OctokitExtensions.cs" />
+    <Compile Include="Application\Organization.cs" />
     <Compile Include="Extensions\ListExtensions.cs" />
     <Compile Include="Git\Tasks\GitLfsVersionTask.cs" />
     <Compile Include="Git\Tasks\GitVersionTask.cs" />
diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/PublishView.cs
@@ -30,7 +30,7 @@ class PublishView : Subview
 
         [SerializeField] private string username;
         [SerializeField] private string[] owners = { OwnersDefaultText };
-        [SerializeField] private IList<Organization> organizations;
+        [SerializeField] private string[] publishOwners;
         [SerializeField] private int selectedOwner;
         [SerializeField] private string repoName = String.Empty;
         [SerializeField] private string repoDescription = "";
@@ -68,7 +68,7 @@ public IApiClient Client
         public override void OnEnable()
         {
             base.OnEnable();
-            ownersNeedLoading = organizations == null && !isBusy;
+            ownersNeedLoading = publishOwners == null && !isBusy;
         }
 
         public override void OnDataUpdate()
@@ -107,14 +107,14 @@ private void LoadOwners()
 
             Client.GetOrganizations(orgs =>
             {
-                organizations = orgs;
-                Logger.Trace("Loaded {0} Owners", organizations.Count);
+                Logger.Trace("Loaded {0} Owners", orgs.Length);
 
-                var organizationLogins = organizations
+                publishOwners = orgs
                     .OrderBy(organization => organization.Login)
-                    .Select(organization => organization.Login);
+                    .Select(organization => organization.Login)
+                    .ToArray();
 
-                owners = new[] { OwnersDefaultText, username }.Union(organizationLogins).ToArray();
+                owners = new[] { OwnersDefaultText, username }.Union(publishOwners).ToArray();
 
                 isBusy = false;
             }, exception =>

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ private async Task LogoutInternal(UriString host)`
`50`	`50`	`await loginManager.Logout(host);`
`51`	`51`	`}`
`52`	`52`
`53`		`- public async Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null)`
	`53`	`+ public async Task CreateRepository(NewRepository newRepository, Action<GitHubRepository, Exception> callback, string organization = null)`
`54`	`54`	`{`
`55`	`55`	`Guard.ArgumentNotNull(callback, "callback");`
`56`	`56`	`try`
`@@ -64,7 +64,7 @@ public async Task CreateRepository(NewRepository newRepository, Action<Octokit.R`
`64`	`64`	`}`
`65`	`65`	`}`
`66`	`66`
`67`		`- public async Task GetOrganizations(Action<IList<Organization>> onSuccess, Action<Exception> onError = null)`
	`67`	`+ public async Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null)`
`68`	`68`	`{`
`69`	`69`	`Guard.ArgumentNotNull(onSuccess, nameof(onSuccess));`
`70`	`70`	`await GetOrganizationInternal(onSuccess, onError);`
`@@ -84,7 +84,7 @@ public async Task ValidateCurrentUser(Action onSuccess, Action<Exception> onErro`
`84`	`84`	`}`
`85`	`85`	`}`
`86`	`86`
`87`		`- public async Task GetCurrentUser(Action<Octokit.User> callback)`
	`87`	`+ public async Task GetCurrentUser(Action<GitHubUser> callback)`
`88`	`88`	`{`
`89`	`89`	`Guard.ArgumentNotNull(callback, "callback");`
`90`	`90`	`var user = await GetCurrentUserInternal();`
`@@ -187,7 +187,7 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe`
`187`	`187`	`return result.Code == LoginResultCodes.Success;`
`188`	`188`	`}`
`189`	`189`
`190`		`- private async Task<Octokit.Repository> CreateRepositoryInternal(NewRepository newRepository, string organization)`
	`190`	`+ private async Task<GitHubRepository> CreateRepositoryInternal(NewRepository newRepository, string organization)`
`191`	`191`	`{`
`192`	`192`	`try`
`193`	`193`	`{`
`@@ -196,18 +196,18 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe`
`196`	`196`	`await ValidateKeychain();`
`197`	`197`	`await ValidateCurrentUserInternal();`
`198`	`198`
`199`		`- Octokit.Repository repository;`
	`199`	`+ GitHubRepository repository;`
`200`	`200`	`if (!string.IsNullOrEmpty(organization))`
`201`	`201`	`{`
`202`	`202`	`logger.Trace("Creating repository for organization");`
`203`	`203`
`204`		`- repository = await githubClient.Repository.Create(organization, newRepository);`
	`204`	`+ repository = (await githubClient.Repository.Create(organization, newRepository)).ToGitHubRepository();`
`205`	`205`	`}`
`206`	`206`	`else`
`207`	`207`	`{`
`208`	`208`	`logger.Trace("Creating repository for user");`
`209`	`209`
`210`		`- repository = await githubClient.Repository.Create(newRepository);`
	`210`	`+ repository = (await githubClient.Repository.Create(newRepository)).ToGitHubRepository();`
`211`	`211`	`}`
`212`	`212`
`213`	`213`	`logger.Trace("Created Repository");`
`@@ -220,7 +220,7 @@ public async Task<bool> ContinueLoginAsync(LoginResult loginResult, Func<LoginRe`
`220`	`220`	`}`
`221`	`221`	`}`
`222`	`222`
`223`		`- private async Task GetOrganizationInternal(Action<IList<Organization>> onSuccess, Action<Exception> onError = null)`
	`223`	`+ private async Task GetOrganizationInternal(Action<Organization[]> onSuccess, Action<Exception> onError = null)`
`224`	`224`	`{`
`225`	`225`	`try`
`226`	`226`	`{`
`@@ -235,7 +235,11 @@ private async Task GetOrganizationInternal(Action<IList<Organization>> onSuccess`
`235`	`235`
`236`	`236`	`if (organizations != null)`
`237`	`237`	`{`
`238`		`- onSuccess(organizations.ToArray());`
	`238`	`+ var array = organizations.Select(organization => new Organization() {`
	`239`	`+ Name = organization.Name,`
	`240`	`+ Login = organization.Login`
	`241`	`+ }).ToArray();`
	`242`	`+ onSuccess(array);`
`239`	`243`	`}`
`240`	`244`	`}`
`241`	`245`	`catch(Exception ex)`
`@@ -245,14 +249,14 @@ private async Task GetOrganizationInternal(Action<IList<Organization>> onSuccess`
`245`	`249`	`}`
`246`	`250`	`}`
`247`	`251`
`248`		`- private async Task<Octokit.User> GetCurrentUserInternal()`
	`252`	`+ private async Task<GitHubUser> GetCurrentUserInternal()`
`249`	`253`	`{`
`250`	`254`	`try`
`251`	`255`	`{`
`252`	`256`	`logger.Trace("Getting Current User");`
`253`	`257`	`await ValidateKeychain();`
`254`	`258`
`255`		`- return await githubClient.User.Current();`
	`259`	`+ return (await githubClient.User.Current()).ToGitHubUser();`
`256`	`260`	`}`
`257`	`261`	`catch (Exception ex)`
`258`	`262`	`{`
`@@ -311,6 +315,18 @@ private async Task ValidateKeychain()`
`311`	`315`	`}`
`312`	`316`	`}`
`313`	`317`
	`318`	`+ class GitHubUser`
	`319`	`+ {`
	`320`	`+ public string Name { get; set; }`
	`321`	`+ public string Login { get; set; }`
	`322`	`+ }`
	`323`	`+`
	`324`	`+ class GitHubRepository`
	`325`	`+ {`
	`326`	`+ public string Name { get; set; }`
	`327`	`+ public string CloneUrl { get; set; }`
	`328`	`+ }`
	`329`	`+`
`314`	`330`	`class ApiClientException : Exception`
`315`	`331`	`{`
`316`	`332`	`public ApiClientException()`
Original file line number	Diff line number	Diff line change
`@@ -9,13 +9,13 @@ interface IApiClient`
`9`	`9`	`{`
`10`	`10`	`HostAddress HostAddress { get; }`
`11`	`11`	`UriString OriginalUrl { get; }`
`12`		`- Task CreateRepository(NewRepository newRepository, Action<Octokit.Repository, Exception> callback, string organization = null);`
`13`		`- Task GetOrganizations(Action<IList<Organization>> onSuccess, Action<Exception> onError = null);`
	`12`	`+ Task CreateRepository(NewRepository newRepository, Action<GitHubRepository, Exception> callback, string organization = null);`
	`13`	`+ Task GetOrganizations(Action<Organization[]> onSuccess, Action<Exception> onError = null);`
`14`	`14`	`Task Login(string username, string password, Action<LoginResult> need2faCode, Action<bool, string> result);`
`15`	`15`	`Task ContinueLogin(LoginResult loginResult, string code);`
`16`	`16`	`Task<bool> LoginAsync(string username, string password, Func<LoginResult, string> need2faCode);`
`17`	`17`	`Task Logout(UriString host);`
`18`		`- Task GetCurrentUser(Action<Octokit.User> callback);`
	`18`	`+ Task GetCurrentUser(Action<GitHubUser> callback);`
`19`	`19`	`Task ValidateCurrentUser(Action onSuccess, Action<Exception> onError = null);`
`20`	`20`	`}`
`21`	`21`	`}`