From 90f67dd37bfade261c2026fb351f6306ab72d167 Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 8 Oct 2013 16:19:08 -0700 Subject: [PATCH] Make credential store awaitable The storage mechanism for credentials is very likely to be an async data store. So might as well play it safe and make it awaitable. --- .../Octokit.Tests.Integration.csproj | 9 +++++++++ Octokit.Tests.Integration/UsersClientTests.cs | 20 +++++++++++++++++++ Octokit.Tests.Integration/packages.config | 3 +++ Octokit.Tests/GitHubClientTests.cs | 3 ++- Octokit/Authentication/Authenticator.cs | 8 +++----- Octokit/GitHubClient.cs | 4 +++- Octokit/Http/Connection.cs | 18 +++++++++++++++-- Octokit/Http/ICredentialStore.cs | 5 ++--- Octokit/Http/InMemoryCredentialStore.cs | 8 +++++--- 9 files changed, 63 insertions(+), 15 deletions(-) diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index b93222e0..7ea68239 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -33,6 +33,15 @@ + + ..\packages\Rx-Core.2.1.30214.0\lib\Net45\System.Reactive.Core.dll + + + ..\packages\Rx-Interfaces.2.1.30214.0\lib\Net45\System.Reactive.Interfaces.dll + + + ..\packages\Rx-Linq.2.1.30214.0\lib\Net45\System.Reactive.Linq.dll + diff --git a/Octokit.Tests.Integration/UsersClientTests.cs b/Octokit.Tests.Integration/UsersClientTests.cs index 36c7d76f..5408a295 100644 --- a/Octokit.Tests.Integration/UsersClientTests.cs +++ b/Octokit.Tests.Integration/UsersClientTests.cs @@ -1,4 +1,5 @@ using System.Net; +using System.Reactive.Linq; using System.Threading.Tasks; using Octokit.Internal; using Octokit.Tests.Helpers; @@ -23,6 +24,25 @@ namespace Octokit.Tests.Integration Assert.Equal("GitHub", user.Company); } + + [IntegrationTest] + public async Task ReturnsSpecifiedUserUsingAwaitableCredentialProvider() + { + var github = new GitHubClient("Octokit Test Runner", new ObservableCredentialProvider()); + + // Get a user by username + var user = await github.User.Get("tclem"); + + Assert.Equal("GitHub", user.Company); + } + + class ObservableCredentialProvider : ICredentialStore + { + public async Task GetCredentials() + { + return await Observable.Return(AutomationSettings.Current.GitHubCredentials); + } + } } public class TheCurrentMethod diff --git a/Octokit.Tests.Integration/packages.config b/Octokit.Tests.Integration/packages.config index 67a23e70..e2fe1ef7 100644 --- a/Octokit.Tests.Integration/packages.config +++ b/Octokit.Tests.Integration/packages.config @@ -1,5 +1,8 @@  + + + \ No newline at end of file diff --git a/Octokit.Tests/GitHubClientTests.cs b/Octokit.Tests/GitHubClientTests.cs index 07a09fda..89f52015 100644 --- a/Octokit.Tests/GitHubClientTests.cs +++ b/Octokit.Tests/GitHubClientTests.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using NSubstitute; using Octokit.Internal; using Xunit; @@ -71,7 +72,7 @@ namespace Octokit.Tests public void IsRetrievedFromCredentialStore() { var credentialStore = Substitute.For(); - credentialStore.GetCredentials().Returns(new Credentials("foo", "bar")); + credentialStore.GetCredentials().Returns(Task.Factory.StartNew(() => new Credentials("foo", "bar"))); var client = new GitHubClient("Test Runner", credentialStore); Assert.Equal("foo", client.Credentials.Login); diff --git a/Octokit/Authentication/Authenticator.cs b/Octokit/Authentication/Authenticator.cs index d8c606a7..bbadae7c 100644 --- a/Octokit/Authentication/Authenticator.cs +++ b/Octokit/Authentication/Authenticator.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Threading.Tasks; namespace Octokit.Internal { @@ -19,14 +20,11 @@ namespace Octokit.Internal CredentialStore = credentialStore; } - public void Apply(IRequest request) + public async Task Apply(IRequest request) { Ensure.ArgumentNotNull(request, "request"); - // TODO: What if the credentials simply don't exist? We should probably - // throw an exception that can be handled and provide guidance to - // ICredentialStore implementors. - var credentials = CredentialStore.GetCredentials() ?? Credentials.Anonymous; + var credentials = await CredentialStore.GetCredentials() ?? Credentials.Anonymous; authenticators[credentials.AuthenticationType].Authenticate(request, credentials); } diff --git a/Octokit/GitHubClient.cs b/Octokit/GitHubClient.cs index fe844f07..ab939b0f 100644 --- a/Octokit/GitHubClient.cs +++ b/Octokit/GitHubClient.cs @@ -47,7 +47,9 @@ namespace Octokit /// Convenience property for getting and setting credentials. /// /// - /// Setting the credentials will change the to use + /// You can use this property if you only have a single hard-coded credential. Otherwise, pass in an + /// to the constructor. + /// Setting this property will change the to use /// the default with just these credentials. /// public Credentials Credentials diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 9e431cee..b104c8c5 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -171,9 +171,23 @@ namespace Octokit.Internal get { return _authenticator.CredentialStore; } } + /// + /// Convenience property for getting and setting credentials. + /// + /// + /// You can use this property if you only have a single hard-coded credential. Otherwise, pass in an + /// to the constructor. + /// Setting this property will change the to use + /// the default with just these credentials. + /// public Credentials Credentials { - get { return CredentialStore.GetCredentials() ?? Credentials.Anonymous; } + get + { + var credentialTask = CredentialStore.GetCredentials(); + if (credentialTask == null) return Credentials.Anonymous; + return credentialTask.Result ?? Credentials.Anonymous; + } // Note this is for convenience. We probably shouldn't allow this to be mutable. set { @@ -200,7 +214,7 @@ namespace Octokit.Internal async Task> RunRequest(IRequest request) { request.Headers.Add("User-Agent", UserAgent); - _authenticator.Apply(request); + await _authenticator.Apply(request); var response = await _httpClient.Send(request); _apiInfoParser.ParseApiHttpHeaders(response); HandleErrors(response); diff --git a/Octokit/Http/ICredentialStore.cs b/Octokit/Http/ICredentialStore.cs index c7993d2e..d53bfe67 100644 --- a/Octokit/Http/ICredentialStore.cs +++ b/Octokit/Http/ICredentialStore.cs @@ -1,11 +1,10 @@ using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; namespace Octokit.Internal { public interface ICredentialStore { - [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate", - Justification = "The credential store migth not be immediate")] - Credentials GetCredentials(); + Task GetCredentials(); } } diff --git a/Octokit/Http/InMemoryCredentialStore.cs b/Octokit/Http/InMemoryCredentialStore.cs index 8df5efe1..56060f1b 100644 --- a/Octokit/Http/InMemoryCredentialStore.cs +++ b/Octokit/Http/InMemoryCredentialStore.cs @@ -1,4 +1,6 @@ -namespace Octokit.Internal +using System.Threading.Tasks; + +namespace Octokit.Internal { public class InMemoryCredentialStore : ICredentialStore { @@ -11,9 +13,9 @@ _credentials = credentials; } - public Credentials GetCredentials() + public Task GetCredentials() { - return _credentials; + return Task.Factory.StartNew(() => _credentials); } } } \ No newline at end of file