From f172edef5bf43340922f116c2d28012d8f781b8c Mon Sep 17 00:00:00 2001 From: Timothy Haagenson Date: Mon, 21 Jul 2014 13:41:34 -0500 Subject: [PATCH 01/54] [WIP] Gists get Commits and Forks - [ ] Finish Gists API Implementation - [ ] [Add method to get gist commits](https://developer.github.com/v3/gists/#list-gist-commits) - [ ] [Add method to get gist forks](https://developer.github.com/v3/gists/#list-gist-forks) Fixes #328, Fixes #216 Added implementation for the remaining pieces of the Gists API. The others mentioned in #328 and #216 were completed through other PRs. --- .../Clients/IObservableGistsClient.cs | 18 ++++++ .../Clients/ObservableGistsClient.cs | 28 +++++++++ .../Clients/GistsClientTests.cs | 12 ++++ Octokit.Tests/Clients/GistsClientTests.cs | 38 +++++++++++++ Octokit.Tests/Octokit.Tests.csproj | 1 + .../Reactive/ObservableGistsTests.cs | 57 +++++++++++++++++++ Octokit/Clients/GistsClient.cs | 28 +++++++++ Octokit/Clients/IGistsClient.cs | 18 ++++++ Octokit/Helpers/ApiUrls.cs | 13 +++++ 9 files changed, 213 insertions(+) create mode 100644 Octokit.Tests/Reactive/ObservableGistsTests.cs diff --git a/Octokit.Reactive/Clients/IObservableGistsClient.cs b/Octokit.Reactive/Clients/IObservableGistsClient.cs index 31ea9da5..88afc627 100644 --- a/Octokit.Reactive/Clients/IObservableGistsClient.cs +++ b/Octokit.Reactive/Clients/IObservableGistsClient.cs @@ -91,6 +91,24 @@ namespace Octokit.Reactive /// Only gists updated at or after this time are returned IObservable GetAllForUser(string user, DateTimeOffset since); + /// + /// List gist commits + /// + /// + /// http://developer.github.com/v3/gists/#list-gists-commits + /// + /// The id of the gist + IObservable GetCommits(string id); + + /// + /// List gist forks + /// + /// + /// http://developer.github.com/v3/gists/#list-gists-forks + /// + /// The id of the gist + IObservable GetForks(string id); + /// /// Creates a new gist /// diff --git a/Octokit.Reactive/Clients/ObservableGistsClient.cs b/Octokit.Reactive/Clients/ObservableGistsClient.cs index 3525b870..755417f0 100644 --- a/Octokit.Reactive/Clients/ObservableGistsClient.cs +++ b/Octokit.Reactive/Clients/ObservableGistsClient.cs @@ -180,6 +180,34 @@ namespace Octokit.Reactive return _connection.GetAndFlattenAllPages(ApiUrls.UsersGists(user), request.ToParametersDictionary()); } + /// + /// List gist commits + /// + /// + /// http://developer.github.com/v3/gists/#list-gists-commits + /// + /// The id of the gist + public IObservable GetCommits(string id) + { + Ensure.ArgumentNotNullOrEmptyString(id, "id"); + + return _connection.GetAndFlattenAllPages(ApiUrls.GistCommits(id)); + } + + /// + /// List gist forks + /// + /// + /// http://developer.github.com/v3/gists/#list-gists-forks + /// + /// The id of the gist + public IObservable GetForks(string id) + { + Ensure.ArgumentNotNullOrEmptyString(id, "id"); + + return _connection.GetAndFlattenAllPages(ApiUrls.ForkGist(id)); + } + /// /// Edits a gist /// diff --git a/Octokit.Tests.Integration/Clients/GistsClientTests.cs b/Octokit.Tests.Integration/Clients/GistsClientTests.cs index b7d1f6bf..28be892e 100644 --- a/Octokit.Tests.Integration/Clients/GistsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/GistsClientTests.cs @@ -126,4 +126,16 @@ public class GistsClientTests await _fixture.Delete(createdGist.Id); } + + [IntegrationTest] + public async Task CanGetGistChildren() + { + // Test History/Commits + var commits = await _fixture.GetCommits(testGistId); + Assert.NotNull(commits); + + // Test Forks + var forks = await _fixture.GetForks(testGistId); + Assert.NotNull(forks); + } } diff --git a/Octokit.Tests/Clients/GistsClientTests.cs b/Octokit.Tests/Clients/GistsClientTests.cs index 53c56a68..aa6dc80a 100644 --- a/Octokit.Tests/Clients/GistsClientTests.cs +++ b/Octokit.Tests/Clients/GistsClientTests.cs @@ -124,6 +124,44 @@ public class GistsClientTests } } + public class TheGetChildrenMethods + { + [Fact] + public void EnsureNonNullArguments() + { + var connection = Substitute.For(); + var client = new GistsClient(connection); + + Assert.Throws(() => client.GetCommits(null)); + Assert.Throws(() => client.GetCommits("")); + + Assert.Throws(() => client.GetForks(null)); + Assert.Throws(() => client.GetForks("")); + } + + [Fact] + public void RequestsCorrectGetCommitsUrl() + { + var connection = Substitute.For(); + var client = new GistsClient(connection); + + client.GetCommits("9257657"); + + connection.Received().GetAll(Arg.Is(u => u.ToString() == "gists/9257657/commits")); + } + + [Fact] + public void RequestsCorrectGetForksUrl() + { + var connection = Substitute.For(); + var client = new GistsClient(connection); + + client.GetForks("9257657"); + + connection.Received().GetAll(Arg.Is(u => u.ToString() == "gists/9257657/forks")); + } + } + public class TheCreateMethod { [Fact] diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index c3477890..f090987f 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -162,6 +162,7 @@ + diff --git a/Octokit.Tests/Reactive/ObservableGistsTests.cs b/Octokit.Tests/Reactive/ObservableGistsTests.cs new file mode 100644 index 00000000..b4dadfb4 --- /dev/null +++ b/Octokit.Tests/Reactive/ObservableGistsTests.cs @@ -0,0 +1,57 @@ +using System; +using System.Collections.Generic; +using System.Reactive.Linq; +using System.Threading.Tasks; +using NSubstitute; +using Octokit; +using Octokit.Internal; +using Octokit.Reactive; +using Octokit.Reactive.Internal; +using Octokit.Tests.Helpers; +using Xunit; +using Xunit.Extensions; + +namespace Octokit.Tests.Reactive +{ + public class ObservableGistsTests + { + public class TheGetChildrenMethods + { + [Fact] + public async Task EnsureNonNullArguments() + { + var client = new ObservableGistsClient(Substitute.For()); + + await AssertEx.Throws(async () => await client.GetCommits(null)); + await AssertEx.Throws(async () => await client.GetCommits("")); + + await AssertEx.Throws(async () => await client.GetForks(null)); + await AssertEx.Throws(async () => await client.GetForks("")); + } + + [Fact] + public void RequestsCorrectGetCommitsUrl() + { + var github = Substitute.For(); + var client = new ObservableGistsClient(github); + var expected = new Uri("gists/9257657/commits", UriKind.Relative); + + client.GetCommits("9257657"); + + github.Connection.Received(1).Get>(expected, Arg.Any>(), null); + } + + [Fact] + public void RequestsCorrectGetForksUrl() + { + var github = Substitute.For(); + var client = new ObservableGistsClient(github); + var expected = new Uri("gists/9257657/forks", UriKind.Relative); + + client.GetForks("9257657"); + + github.Connection.Received(1).Get>(expected, Arg.Any>(), null); + } + } + } +} \ No newline at end of file diff --git a/Octokit/Clients/GistsClient.cs b/Octokit/Clients/GistsClient.cs index fcafeb79..124380d8 100644 --- a/Octokit/Clients/GistsClient.cs +++ b/Octokit/Clients/GistsClient.cs @@ -196,6 +196,34 @@ namespace Octokit return ApiConnection.GetAll(ApiUrls.UsersGists(user), request.ToParametersDictionary()); } + /// + /// List gist commits + /// + /// + /// http://developer.github.com/v3/gists/#list-gists-commits + /// + /// The id of the gist + public Task> GetCommits(string id) + { + Ensure.ArgumentNotNullOrEmptyString(id, "id"); + + return ApiConnection.GetAll(ApiUrls.GistCommits(id)); + } + + /// + /// List gist forks + /// + /// + /// http://developer.github.com/v3/gists/#list-gists-forks + /// + /// The id of the gist + public Task> GetForks(string id) + { + Ensure.ArgumentNotNullOrEmptyString(id, "id"); + + return ApiConnection.GetAll(ApiUrls.ForkGist(id)); + } + /// /// Edits a gist /// diff --git a/Octokit/Clients/IGistsClient.cs b/Octokit/Clients/IGistsClient.cs index c344c576..476a1fb2 100644 --- a/Octokit/Clients/IGistsClient.cs +++ b/Octokit/Clients/IGistsClient.cs @@ -98,6 +98,24 @@ namespace Octokit /// Only gists updated at or after this time are returned Task> GetAllForUser(string user, DateTimeOffset since); + /// + /// List gist commits + /// + /// + /// http://developer.github.com/v3/gists/#list-gists-commits + /// + /// The id of the gist + Task> GetCommits(string id); + + /// + /// List gist forks + /// + /// + /// http://developer.github.com/v3/gists/#list-gists-forks + /// + /// The id of the gist + Task> GetForks(string id); + /// /// Creates a new gist /// diff --git a/Octokit/Helpers/ApiUrls.cs b/Octokit/Helpers/ApiUrls.cs index 925f40f1..929d9054 100644 --- a/Octokit/Helpers/ApiUrls.cs +++ b/Octokit/Helpers/ApiUrls.cs @@ -646,6 +646,10 @@ namespace Octokit return "gists/{0}".FormatUri(id); } + /// + /// Returns the for the forks for the specified gist. + /// + /// The id of the gist public static Uri ForkGist(string id) { return "gists/{0}/forks".FormatUri(id); @@ -680,6 +684,15 @@ namespace Octokit return "gists/{0}/comments".FormatUri(gistId); } + /// + /// Returns the for the commits for the specified gist. + /// + /// The id of the gist + public static Uri GistCommits(string id) + { + return "gists/{0}/commits".FormatUri(id); + } + /// /// Returns the that returns the specified pull request. /// From e8d894c648f6d0e472ad324b73de37575b4c5466 Mon Sep 17 00:00:00 2001 From: Timothy Haagenson Date: Mon, 18 Aug 2014 17:15:31 -0400 Subject: [PATCH 02/54] small changes to make ready for master a few small fixes added debugger attributes to models being used and added back a missing newline at end of file fixed a quick typo on the documentation for GistChangeStatus --- Octokit.Tests/Reactive/ObservableGistsTests.cs | 2 +- Octokit/Models/Response/GistChangeStatus.cs | 2 +- Octokit/Models/Response/GistFork.cs | 11 +++++++++++ Octokit/Models/Response/GistHistory.cs | 11 +++++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/Octokit.Tests/Reactive/ObservableGistsTests.cs b/Octokit.Tests/Reactive/ObservableGistsTests.cs index b4dadfb4..c8519a47 100644 --- a/Octokit.Tests/Reactive/ObservableGistsTests.cs +++ b/Octokit.Tests/Reactive/ObservableGistsTests.cs @@ -54,4 +54,4 @@ namespace Octokit.Tests.Reactive } } } -} \ No newline at end of file +} diff --git a/Octokit/Models/Response/GistChangeStatus.cs b/Octokit/Models/Response/GistChangeStatus.cs index 4b790347..9601fd2e 100644 --- a/Octokit/Models/Response/GistChangeStatus.cs +++ b/Octokit/Models/Response/GistChangeStatus.cs @@ -1,7 +1,7 @@ namespace Octokit { /// - /// User by to indicate the level of change. + /// Used by to indicate the level of change. /// public class GistChangeStatus { diff --git a/Octokit/Models/Response/GistFork.cs b/Octokit/Models/Response/GistFork.cs index b6c3662c..84bf95cd 100644 --- a/Octokit/Models/Response/GistFork.cs +++ b/Octokit/Models/Response/GistFork.cs @@ -1,7 +1,10 @@ using System; +using System.Diagnostics; +using System.Globalization; namespace Octokit { + [DebuggerDisplay("{DebuggerDisplay,nq}")] public class GistFork { /// @@ -18,5 +21,13 @@ namespace Octokit /// The for when this was created. /// public DateTimeOffset CreatedAt { get; set; } + + internal string DebuggerDisplay + { + get + { + return String.Format(CultureInfo.InvariantCulture, "Url: {0}", Url); + } + } } } \ No newline at end of file diff --git a/Octokit/Models/Response/GistHistory.cs b/Octokit/Models/Response/GistHistory.cs index 0ea5fe32..6218321a 100644 --- a/Octokit/Models/Response/GistHistory.cs +++ b/Octokit/Models/Response/GistHistory.cs @@ -1,10 +1,13 @@ using System; +using System.Diagnostics; +using System.Globalization; namespace Octokit { /// /// A historical version of a /// + [DebuggerDisplay("{DebuggerDisplay,nq}")] public class GistHistory { /// @@ -31,5 +34,13 @@ namespace Octokit /// The the version was created. /// public DateTimeOffset CommittedAt { get; set; } + + internal string DebuggerDisplay + { + get + { + return String.Format(CultureInfo.InvariantCulture, "Url: {0} | Version: {1}", Url, Version); + } + } } } \ No newline at end of file From 93dd16f8296fae0d55c84534f9a6b050e5b54cc9 Mon Sep 17 00:00:00 2001 From: Haacked Date: Sun, 19 Apr 2015 19:42:13 -0700 Subject: [PATCH 03/54] Add more info to TwoFactorChallengeFailedException I was running into an issue where i wanted more information from the TwoFactorChallengeFailedException. It turns out, the exception could easily provide both the TwoFactorType AND the authentication code provided. So :boom:! --- Octokit/Clients/AuthorizationsClient.cs | 2 +- Octokit/Exceptions/ApiException.cs | 3 + .../TwoFactorAuthorizationException.cs | 115 ++++++++++++++++++ .../TwoFactorChallengeFailedException.cs | 31 +++-- .../Exceptions/TwoFactorRequiredException.cs | 48 +------- Octokit/Http/Connection.cs | 4 +- Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-MonoAndroid.csproj | 1 + Octokit/Octokit-Monotouch.csproj | 1 + Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 1 + Octokit/Octokit.csproj | 1 + 12 files changed, 155 insertions(+), 54 deletions(-) create mode 100644 Octokit/Exceptions/TwoFactorAuthorizationException.cs diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 029cc313..9d4c342a 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -172,7 +172,7 @@ namespace Octokit } catch (AuthorizationException e) { - throw new TwoFactorChallengeFailedException(e); + throw new TwoFactorChallengeFailedException(twoFactorAuthenticationCode, e); } } diff --git a/Octokit/Exceptions/ApiException.cs b/Octokit/Exceptions/ApiException.cs index 9e16c540..fa30f9f2 100644 --- a/Octokit/Exceptions/ApiException.cs +++ b/Octokit/Exceptions/ApiException.cs @@ -67,6 +67,7 @@ namespace Octokit StatusCode = response.StatusCode; ApiError = GetApiErrorFromExceptionMessage(response); + HttpResponse = response; } /// @@ -97,6 +98,8 @@ namespace Octokit StatusCode = statusCode; } + public IResponse HttpResponse { get; private set; } + public override string Message { get { return ApiErrorMessageSafe ?? "An error occurred with this API request"; } diff --git a/Octokit/Exceptions/TwoFactorAuthorizationException.cs b/Octokit/Exceptions/TwoFactorAuthorizationException.cs new file mode 100644 index 00000000..beae116c --- /dev/null +++ b/Octokit/Exceptions/TwoFactorAuthorizationException.cs @@ -0,0 +1,115 @@ +using System; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Net; +using System.Runtime.Serialization; + +namespace Octokit +{ +#if !NETFX_CORE + /// + /// Represents a failed 2FA challenge from the API + /// + [Serializable] +#endif + [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", + Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] + public abstract class TwoFactorAuthorizationException : AuthorizationException + { + /// + /// Constructs an instance of TwoFactorRequiredException. + /// + /// Expected 2FA response type + /// The inner exception + protected TwoFactorAuthorizationException(TwoFactorType twoFactorType, Exception innerException) + : base(HttpStatusCode.Unauthorized, innerException) + { + TwoFactorType = twoFactorType; + } + + /// + /// Constructs an instance of TwoFactorRequiredException. + /// + /// The HTTP payload from the server + /// Expected 2FA response type + protected TwoFactorAuthorizationException(IResponse response, TwoFactorType twoFactorType) + : base(response) + { + Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Unauthorized, + "TwoFactorRequiredException status code should be 401"); + + TwoFactorType = twoFactorType; + } + + /// + /// Constructs an instance of TwoFactorRequiredException. + /// + /// The HTTP payload from the server + /// Expected 2FA response type + /// The inner exception + protected TwoFactorAuthorizationException(IResponse response, TwoFactorType twoFactorType, Exception innerException) + : base(response, innerException) + { + Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Unauthorized, + "TwoFactorRequiredException status code should be 401"); + + TwoFactorType = twoFactorType; + } + + /// + /// Expected 2FA response type + /// + public TwoFactorType TwoFactorType { get; private set; } + + #if !NETFX_CORE + /// + /// Constructs an instance of TwoFactorRequiredException. + /// + /// + /// The that holds the + /// serialized object data about the exception being thrown. + /// + /// + /// The that contains + /// contextual information about the source or destination. + /// + protected TwoFactorAuthorizationException(SerializationInfo info, StreamingContext context) + : base(info, context) + { + if (info == null) return; + TwoFactorType = (TwoFactorType) (info.GetInt32("TwoFactorType")); + } + + public override void GetObjectData(SerializationInfo info, StreamingContext context) + { + base.GetObjectData(info, context); + info.AddValue("TwoFactorType", TwoFactorType); + } +#endif + + } + + /// + /// Methods for receiving 2FA authentication codes + /// + public enum TwoFactorType + { + /// + /// No method configured + /// + None, + /// + /// Unknown method + /// + Unknown, + /// + /// Receive via SMS + /// + [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Sms")] + Sms, + /// + /// Receive via application + /// + AuthenticatorApp + } +} diff --git a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs index 7a93bbae..e068f206 100644 --- a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs +++ b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs @@ -1,6 +1,5 @@ using System; using System.Diagnostics.CodeAnalysis; -using System.Net; using System.Runtime.Serialization; namespace Octokit @@ -13,23 +12,26 @@ namespace Octokit #endif [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] - public class TwoFactorChallengeFailedException : AuthorizationException + public class TwoFactorChallengeFailedException : TwoFactorAuthorizationException { /// /// Constructs an instance of TwoFactorChallengeFailedException /// - public TwoFactorChallengeFailedException() : - base(HttpStatusCode.Unauthorized, null) + public TwoFactorChallengeFailedException() : base(TwoFactorType.None, null) { } /// /// Constructs an instance of TwoFactorChallengeFailedException /// + /// The authorization code that was incorrect /// The inner exception - public TwoFactorChallengeFailedException(Exception innerException) - : base(HttpStatusCode.Unauthorized, innerException) + public TwoFactorChallengeFailedException( + string authorizationCode, + ApiException innerException) + : base(ParseTwoFactorType(innerException), innerException) { + AuthorizationCode = authorizationCode; } public override string Message @@ -37,9 +39,16 @@ namespace Octokit get { return "The two-factor authentication code supplied is not correct"; } } + public string AuthorizationCode { get; private set; } + + static TwoFactorType ParseTwoFactorType(ApiException exception) + { + return exception == null ? TwoFactorType.None : Connection.ParseTwoFactorType(exception.HttpResponse); + } + #if !NETFX_CORE /// - /// Constructs an instance of TwoFactorChallengeFailedException + /// Constructs an instance of TwoFactorRequiredException. /// /// /// The that holds the @@ -52,6 +61,14 @@ namespace Octokit protected TwoFactorChallengeFailedException(SerializationInfo info, StreamingContext context) : base(info, context) { + if (info == null) return; + AuthorizationCode = info.GetString("AuthorizationCode"); + } + + public override void GetObjectData(SerializationInfo info, StreamingContext context) + { + base.GetObjectData(info, context); + info.AddValue("AuthorizationCode", AuthorizationCode); } #endif } diff --git a/Octokit/Exceptions/TwoFactorRequiredException.cs b/Octokit/Exceptions/TwoFactorRequiredException.cs index ad7a2ee3..f2361c24 100644 --- a/Octokit/Exceptions/TwoFactorRequiredException.cs +++ b/Octokit/Exceptions/TwoFactorRequiredException.cs @@ -14,7 +14,7 @@ namespace Octokit #endif [SuppressMessage("Microsoft.Design", "CA1032:ImplementStandardExceptionConstructors", Justification = "These exceptions are specific to the GitHub API and not general purpose exceptions")] - public class TwoFactorRequiredException : AuthorizationException + public class TwoFactorRequiredException : TwoFactorAuthorizationException { /// /// Constructs an instance of TwoFactorRequiredException. @@ -27,10 +27,8 @@ namespace Octokit /// Constructs an instance of TwoFactorRequiredException. /// /// Expected 2FA response type - public TwoFactorRequiredException(TwoFactorType twoFactorType) - : base(HttpStatusCode.Unauthorized, null) + public TwoFactorRequiredException(TwoFactorType twoFactorType) : base(twoFactorType, null) { - TwoFactorType = twoFactorType; } /// @@ -38,12 +36,11 @@ namespace Octokit /// /// The HTTP payload from the server /// Expected 2FA response type - public TwoFactorRequiredException(IResponse response, TwoFactorType twoFactorType) : base(response) + public TwoFactorRequiredException(IResponse response, TwoFactorType twoFactorType) + : base(response, twoFactorType) { Debug.Assert(response != null && response.StatusCode == HttpStatusCode.Unauthorized, "TwoFactorRequiredException status code should be 401"); - - TwoFactorType = twoFactorType; } public override string Message @@ -66,44 +63,7 @@ namespace Octokit protected TwoFactorRequiredException(SerializationInfo info, StreamingContext context) : base(info, context) { - if (info == null) return; - TwoFactorType = (TwoFactorType) (info.GetInt32("TwoFactorType")); - } - - public override void GetObjectData(SerializationInfo info, StreamingContext context) - { - base.GetObjectData(info, context); - info.AddValue("TwoFactorType", TwoFactorType); } #endif - - /// - /// Expected 2FA response type - /// - public TwoFactorType TwoFactorType { get; private set; } - } - - /// - /// Methods for receiving 2FA authentication codes - /// - public enum TwoFactorType - { - /// - /// No method configured - /// - None, - /// - /// Unknown method - /// - Unknown, - /// - /// Receive via SMS - /// - [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Sms")] - Sms, - /// - /// Receive via application - /// - AuthenticatorApp } } diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 43a808c2..29dc6f1b 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -489,9 +489,9 @@ namespace Octokit : new ForbiddenException(response); } - static TwoFactorType ParseTwoFactorType(IResponse restResponse) + internal static TwoFactorType ParseTwoFactorType(IResponse restResponse) { - if (restResponse.Headers == null || !restResponse.Headers.Any()) return TwoFactorType.None; + if (restResponse == null || restResponse.Headers == null || !restResponse.Headers.Any()) return TwoFactorType.None; var otpHeader = restResponse.Headers.FirstOrDefault(header => header.Key.Equals("X-GitHub-OTP", StringComparison.OrdinalIgnoreCase)); if (String.IsNullOrEmpty(otpHeader.Value)) return TwoFactorType.None; diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index d9f81fc2..3f0dc203 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -383,6 +383,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 897b2a73..2dbbd085 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -395,6 +395,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 997f94e7..13f7b0cf 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -388,6 +388,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 55d10491..68a6e7dd 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -381,6 +381,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index 65291e0a..44206c7f 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -385,6 +385,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index fdf8c32f..55830cee 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -73,6 +73,7 @@ + From 2e2009f6e8d01855c508fd921eecb2d0830bfbfa Mon Sep 17 00:00:00 2001 From: Haacked Date: Sun, 19 Apr 2015 22:20:52 -0700 Subject: [PATCH 04/54] Fix some typos --- Octokit/Exceptions/TwoFactorAuthorizationException.cs | 2 +- Octokit/Exceptions/TwoFactorChallengeFailedException.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Octokit/Exceptions/TwoFactorAuthorizationException.cs b/Octokit/Exceptions/TwoFactorAuthorizationException.cs index beae116c..c8f16cb2 100644 --- a/Octokit/Exceptions/TwoFactorAuthorizationException.cs +++ b/Octokit/Exceptions/TwoFactorAuthorizationException.cs @@ -61,7 +61,7 @@ namespace Octokit /// public TwoFactorType TwoFactorType { get; private set; } - #if !NETFX_CORE +#if !NETFX_CORE /// /// Constructs an instance of TwoFactorRequiredException. /// diff --git a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs index e068f206..d501fe18 100644 --- a/Octokit/Exceptions/TwoFactorChallengeFailedException.cs +++ b/Octokit/Exceptions/TwoFactorChallengeFailedException.cs @@ -48,7 +48,7 @@ namespace Octokit #if !NETFX_CORE /// - /// Constructs an instance of TwoFactorRequiredException. + /// Constructs an instance of TwoFactorChallengeFailedException. /// /// /// The that holds the From 4ca3db562a62a0336bff39b84d94831e02c09456 Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 13:19:21 -0700 Subject: [PATCH 05/54] :fire: Remove authorizations preview accepts header This is now public --- Octokit/Clients/AuthorizationsClient.cs | 89 +++++++++++-------------- 1 file changed, 39 insertions(+), 50 deletions(-) diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 9d4c342a..86bd845c 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -14,8 +14,6 @@ namespace Octokit /// public class AuthorizationsClient : ApiClient, IAuthorizationsClient { - const string previewAcceptsHeader = "application/vnd.github.mirage-preview+json"; - /// /// Initializes a new GitHub OAuth API client. /// @@ -38,7 +36,7 @@ namespace Octokit /// A list of s. public Task> GetAll() { - return ApiConnection.GetAll(ApiUrls.Authorizations(), null, previewAcceptsHeader); + return ApiConnection.GetAll(ApiUrls.Authorizations(), null); } /// @@ -56,7 +54,7 @@ namespace Octokit /// The specified . public Task Get(int id) { - return ApiConnection.Get(ApiUrls.Authorizations(id), null, previewAcceptsHeader); + return ApiConnection.Get(ApiUrls.Authorizations(id), null); } /// @@ -95,18 +93,11 @@ namespace Octokit note_url = newAuthorization.NoteUrl }; - if (String.IsNullOrWhiteSpace(newAuthorization.Fingerprint)) - { - // use classic API - var endpoint = ApiUrls.AuthorizationsForClient(clientId); - return ApiConnection.Put(endpoint, requestData); - } - else - { - // use new API - var endpoint = ApiUrls.AuthorizationsForClient(clientId, newAuthorization.Fingerprint); - return ApiConnection.Put(endpoint, requestData, null, previewAcceptsHeader); - } + var endpoint = string.IsNullOrWhiteSpace(newAuthorization.Fingerprint) + ? ApiUrls.AuthorizationsForClient(clientId) + : ApiUrls.AuthorizationsForClient(clientId, newAuthorization.Fingerprint); + + return ApiConnection.Put(endpoint, requestData); } /// @@ -150,25 +141,14 @@ namespace Octokit try { - if (String.IsNullOrWhiteSpace(newAuthorization.Fingerprint)) - { - // use classic API - var endpoint = ApiUrls.AuthorizationsForClient(clientId); - return await ApiConnection.Put( - endpoint, - requestData, - twoFactorAuthenticationCode); - } - else - { - // use new API - var endpoint = ApiUrls.AuthorizationsForClient(clientId, newAuthorization.Fingerprint); - return await ApiConnection.Put( - endpoint, - requestData, - twoFactorAuthenticationCode, - previewAcceptsHeader); - } + var endpoint = string.IsNullOrWhiteSpace(newAuthorization.Fingerprint) + ? ApiUrls.AuthorizationsForClient(clientId) + : ApiUrls.AuthorizationsForClient(clientId, newAuthorization.Fingerprint); + + return await ApiConnection.Put( + endpoint, + requestData, + twoFactorAuthenticationCode); } catch (AuthorizationException e) { @@ -194,8 +174,7 @@ namespace Octokit var endpoint = ApiUrls.ApplicationAuthorization(clientId, accessToken); return await ApiConnection.Get( endpoint, - null, - previewAcceptsHeader); + null); } /// @@ -274,19 +253,9 @@ namespace Octokit { Ensure.ArgumentNotNull(authorizationUpdate, "authorizationUpdate"); - if (String.IsNullOrWhiteSpace(authorizationUpdate.Fingerprint)) - { - return ApiConnection.Patch( - ApiUrls.Authorizations(id), - authorizationUpdate); - } - else - { - return ApiConnection.Patch( - ApiUrls.Authorizations(id), - authorizationUpdate, - previewAcceptsHeader); - } + return ApiConnection.Patch( + ApiUrls.Authorizations(id), + authorizationUpdate); } /// @@ -307,5 +276,25 @@ namespace Octokit { return ApiConnection.Delete(ApiUrls.Authorizations(id)); } + + /// + /// Deletes the specified . + /// + /// + /// This method requires authentication. + /// See the API + /// documentation for more details. + /// + /// The system-wide ID of the authorization to delete + /// Two factor authorization code + /// + /// Thrown when the current user does not have permission to make the request. + /// + /// Thrown when a general API error occurs. + /// A for the request's execution. + public Task Delete(int id, string twoFactorAuthenticationCode) + { + return ApiConnection.Delete(ApiUrls.Authorizations(id), twoFactorAuthenticationCode); + } } } From 23bb76a1127b22b32bab9c87cbf7a566a6ef18b5 Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 13:40:26 -0700 Subject: [PATCH 06/54] Implement Authorizations Delete method with 2fa code When we delete an authorization, we often need to use basic authentication because the client might not have the associated token stored anymore. If 2fa is enabled, the client needs to be able to pass that along. --- .../IObservableAuthorizationsClient.cs | 30 +++++++++++++++-- .../Clients/ObservableAuthorizationsClient.cs | 33 +++++++++++++++++-- Octokit/Clients/IAuthorizationsClient.cs | 17 ++++++++++ Octokit/Http/ApiConnection.cs | 13 ++++++++ Octokit/Http/Connection.cs | 20 +++++++++++ Octokit/Http/IApiConnection.cs | 8 +++++ Octokit/Http/IConnection.cs | 8 +++++ 7 files changed, 123 insertions(+), 6 deletions(-) diff --git a/Octokit.Reactive/Clients/IObservableAuthorizationsClient.cs b/Octokit.Reactive/Clients/IObservableAuthorizationsClient.cs index 5109bc2c..48847f87 100644 --- a/Octokit.Reactive/Clients/IObservableAuthorizationsClient.cs +++ b/Octokit.Reactive/Clients/IObservableAuthorizationsClient.cs @@ -133,10 +133,34 @@ namespace Octokit.Reactive IObservable Update(int id, AuthorizationUpdate authorizationUpdate); /// - /// Deletes an . + /// Deletes the specified . /// - /// The systemwide id of the authorization - /// + /// + /// This method requires authentication. + /// See the API + /// documentation for more details. + /// + /// The system-wide ID of the authorization to delete + /// + /// Thrown when the current user does not have permission to make the request. + /// + /// Thrown when a general API error occurs. IObservable Delete(int id); + + /// + /// Deletes the specified . + /// + /// + /// This method requires authentication. + /// See the API + /// documentation for more details. + /// + /// The system-wide ID of the authorization to delete + /// Two factor authorization code + /// + /// Thrown when the current user does not have permission to make the request. + /// + /// Thrown when a general API error occurs. + IObservable Delete(int id, string twoFactorAuthenticationCode); } } diff --git a/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs b/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs index 82453680..6b0bd124 100644 --- a/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs +++ b/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs @@ -201,13 +201,40 @@ namespace Octokit.Reactive } /// - /// Deletes an . + /// Deletes the specified . /// - /// The systemwide id of the authorization - /// + /// + /// This method requires authentication. + /// See the API + /// documentation for more details. + /// + /// The system-wide ID of the authorization to delete + /// + /// Thrown when the current user does not have permission to make the request. + /// + /// Thrown when a general API error occurs. public IObservable Delete(int id) { return _client.Delete(id).ToObservable(); } + + /// + /// Deletes the specified . + /// + /// + /// This method requires authentication. + /// See the API + /// documentation for more details. + /// + /// The system-wide ID of the authorization to delete + /// Two factor authorization code + /// + /// Thrown when the current user does not have permission to make the request. + /// + /// Thrown when a general API error occurs. + public IObservable Delete(int id, string twoFactorAuthenticationCode) + { + return _client.Delete(id, twoFactorAuthenticationCode).ToObservable(); + } } } diff --git a/Octokit/Clients/IAuthorizationsClient.cs b/Octokit/Clients/IAuthorizationsClient.cs index 583bcd89..9d050d54 100644 --- a/Octokit/Clients/IAuthorizationsClient.cs +++ b/Octokit/Clients/IAuthorizationsClient.cs @@ -178,5 +178,22 @@ namespace Octokit /// Thrown when a general API error occurs. /// A for the request's execution. Task Delete(int id); + + /// + /// Deletes the specified . + /// + /// + /// This method requires authentication. + /// See the API + /// documentation for more details. + /// + /// The system-wide ID of the authorization to delete + /// Two factor authorization code + /// + /// Thrown when the current user does not have permission to make the request. + /// + /// Thrown when a general API error occurs. + /// A for the request's execution. + Task Delete(int id, string twoFactorAuthenticationCode); } } diff --git a/Octokit/Http/ApiConnection.cs b/Octokit/Http/ApiConnection.cs index dc6881dc..51113271 100644 --- a/Octokit/Http/ApiConnection.cs +++ b/Octokit/Http/ApiConnection.cs @@ -332,6 +332,19 @@ namespace Octokit return Connection.Delete(uri); } + /// + /// Deletes the API object at the specified URI. + /// + /// URI of the API resource to delete + /// Two Factor Code + /// A for the request's execution. + public Task Delete(Uri uri, string twoFactorAuthenticationCode) + { + Ensure.ArgumentNotNull(uri, "uri"); + + return Connection.Delete(uri, twoFactorAuthenticationCode); + } + /// /// Deletes the API object at the specified URI. /// diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 29dc6f1b..a4c88909 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -361,6 +361,26 @@ namespace Octokit return response.HttpResponse.StatusCode; } + /// + /// Performs an asynchronous HTTP DELETE request that expects an empty response. + /// + /// URI endpoint to send request to + /// Two Factor Code + /// The returned + public async Task Delete(Uri uri, string twoFactorAuthenticationCode) + { + Ensure.ArgumentNotNull(uri, "uri"); + + var response = await SendData( + uri, + HttpMethod.Delete, + null, + null, + CancellationToken.None, + twoFactorAuthenticationCode); + return response.HttpResponse.StatusCode; + } + /// /// Performs an asynchronous HTTP DELETE request that expects an empty response. /// diff --git a/Octokit/Http/IApiConnection.cs b/Octokit/Http/IApiConnection.cs index 18416063..5bf11f8b 100644 --- a/Octokit/Http/IApiConnection.cs +++ b/Octokit/Http/IApiConnection.cs @@ -203,6 +203,14 @@ namespace Octokit /// A for the request's execution. Task Delete(Uri uri); + /// + /// Deletes the API object at the specified URI. + /// + /// URI of the API resource to delete + /// Two Factor Code + /// A for the request's execution. + Task Delete(Uri uri, string twoFactorAuthenticationCode); + /// /// Deletes the API object at the specified URI. /// diff --git a/Octokit/Http/IConnection.cs b/Octokit/Http/IConnection.cs index 41b77e86..37a6c004 100644 --- a/Octokit/Http/IConnection.cs +++ b/Octokit/Http/IConnection.cs @@ -164,6 +164,14 @@ namespace Octokit /// The returned Task Delete(Uri uri); + /// + /// Performs an asynchronous HTTP DELETE request that expects an empty response. + /// + /// URI endpoint to send request to + /// Two Factor Code + /// The returned + Task Delete(Uri uri, string twoFactorAuthenticationCode); + /// /// Performs an asynchronous HTTP DELETE request that expects an empty response. /// From 408e95c421f16cc673bcb5efae895c96635269d4 Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 13:51:25 -0700 Subject: [PATCH 07/54] Implement Authorization Create method --- .../IObservableAuthorizationsClient.cs | 50 +++++++++++ .../Clients/ObservableAuthorizationsClient.cs | 65 ++++++++++++++ Octokit/Clients/AuthorizationsClient.cs | 89 +++++++++++++++++++ Octokit/Clients/IAuthorizationsClient.cs | 50 +++++++++++ Octokit/Http/Connection.cs | 1 + 5 files changed, 255 insertions(+) diff --git a/Octokit.Reactive/Clients/IObservableAuthorizationsClient.cs b/Octokit.Reactive/Clients/IObservableAuthorizationsClient.cs index 48847f87..a61641d1 100644 --- a/Octokit.Reactive/Clients/IObservableAuthorizationsClient.cs +++ b/Octokit.Reactive/Clients/IObservableAuthorizationsClient.cs @@ -31,6 +31,56 @@ namespace Octokit.Reactive Justification = "It's fiiiine. It's fine. Trust us.")] IObservable Get(int id); + /// + /// Creates a new authorization for the specified OAuth application if an authorization for that application + /// doesn’t already exist for the user; otherwise, it fails. + /// + /// + /// This method requires authentication. + /// See the API documentation for more information. + /// + /// Client ID of the OAuth application for the token + /// The client secret + /// Describes the new authorization to create + /// + /// Thrown when the current user does not have permission to make this request. + /// + /// + /// Thrown when the current account has two-factor authentication enabled and an authentication code is required. + /// + /// Thrown when a general API error occurs. + /// The created . + IObservable Create( + string clientId, + string clientSecret, + NewAuthorization newAuthorization); + + /// + /// Creates a new authorization for the specified OAuth application if an authorization for that application + /// doesn’t already exist for the user; otherwise, it fails. + /// + /// + /// This method requires authentication. + /// See the API documentation for more information. + /// + /// Client ID of the OAuth application for the token + /// The client secret + /// The two-factor authentication code in response to the current user's previous challenge + /// Describes the new authorization to create + /// + /// Thrown when the current user does not have permission to make this request. + /// + /// + /// Thrown when the current account has two-factor authentication enabled and an authentication code is required. + /// + /// Thrown when a general API error occurs. + /// The created . + IObservable Create( + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + string twoFactorAuthenticationCode); + /// /// This method will create a new authorization for the specified OAuth application, only if an authorization /// for that application doesn’t already exist for the user. It returns the user’s token for the application diff --git a/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs b/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs index 6b0bd124..7192412d 100644 --- a/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs +++ b/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs @@ -45,6 +45,71 @@ namespace Octokit.Reactive return _client.Get(id).ToObservable(); } + /// + /// Creates a new authorization for the specified OAuth application if an authorization for that application + /// doesn’t already exist for the user; otherwise, it fails. + /// + /// + /// This method requires authentication. + /// See the API documentation for more information. + /// + /// Client ID of the OAuth application for the token + /// The client secret + /// Describes the new authorization to create + /// + /// Thrown when the current user does not have permission to make this request. + /// + /// + /// Thrown when the current account has two-factor authentication enabled and an authentication code is required. + /// + /// Thrown when a general API error occurs. + /// The created . + public IObservable Create( + string clientId, + string clientSecret, + NewAuthorization newAuthorization) + { + Ensure.ArgumentNotNullOrEmptyString(clientId, "clientId"); + Ensure.ArgumentNotNullOrEmptyString(clientSecret, "clientSecret"); + Ensure.ArgumentNotNull(newAuthorization, "authorization"); + + return _client.Create(clientId, clientSecret, newAuthorization).ToObservable(); + } + + /// + /// Creates a new authorization for the specified OAuth application if an authorization for that application + /// doesn’t already exist for the user; otherwise, it fails. + /// + /// + /// This method requires authentication. + /// See the API documentation for more information. + /// + /// Client ID of the OAuth application for the token + /// The client secret + /// The two-factor authentication code in response to the current user's previous challenge + /// Describes the new authorization to create + /// + /// Thrown when the current user does not have permission to make this request. + /// + /// + /// Thrown when the current account has two-factor authentication enabled and an authentication code is required. + /// + /// Thrown when a general API error occurs. + /// The created . + public IObservable Create( + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + string twoFactorAuthenticationCode) + { + Ensure.ArgumentNotNullOrEmptyString(clientId, "clientId"); + Ensure.ArgumentNotNullOrEmptyString(clientSecret, "clientSecret"); + Ensure.ArgumentNotNull(newAuthorization, "authorization"); + Ensure.ArgumentNotNullOrEmptyString(twoFactorAuthenticationCode, "twoFactorAuthenticationCode"); + + return _client.Create(clientId, clientSecret, newAuthorization).ToObservable(); + } + /// /// This method will create a new authorization for the specified OAuth application, only if an authorization /// for that application doesn’t already exist for the user. It returns the user’s token for the application diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 86bd845c..73fd742b 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -57,6 +57,95 @@ namespace Octokit return ApiConnection.Get(ApiUrls.Authorizations(id), null); } + /// + /// Creates a new authorization for the specified OAuth application if an authorization for that application + /// doesn’t already exist for the user; otherwise, it fails. + /// + /// + /// This method requires authentication. + /// See the API documentation for more information. + /// + /// Client ID of the OAuth application for the token + /// The client secret + /// Describes the new authorization to create + /// + /// Thrown when the current user does not have permission to make this request. + /// + /// + /// Thrown when the current account has two-factor authentication enabled and an authentication code is required. + /// + /// Thrown when a general API error occurs. + /// The created . + public Task Create( + string clientId, + string clientSecret, + NewAuthorization newAuthorization) + { + Ensure.ArgumentNotNullOrEmptyString(clientId, "clientId"); + Ensure.ArgumentNotNullOrEmptyString(clientSecret, "clientSecret"); + Ensure.ArgumentNotNull(newAuthorization, "authorization"); + + var requestData = new + { + client_id = clientId, + client_secret = clientSecret, + scopes = newAuthorization.Scopes, + note = newAuthorization.Note, + note_url = newAuthorization.NoteUrl, + fingerprint = newAuthorization.Fingerprint + }; + + var endpoint = ApiUrls.Authorizations(); + + return ApiConnection.Put(endpoint, requestData); + } + + /// + /// Creates a new authorization for the specified OAuth application if an authorization for that application + /// doesn’t already exist for the user; otherwise, it fails. + /// + /// + /// This method requires authentication. + /// See the API documentation for more information. + /// + /// Client ID of the OAuth application for the token + /// The client secret + /// The two-factor authentication code in response to the current user's previous challenge + /// Describes the new authorization to create + /// + /// Thrown when the current user does not have permission to make this request. + /// + /// + /// Thrown when the current account has two-factor authentication enabled and an authentication code is required. + /// + /// Thrown when a general API error occurs. + /// The created . + public Task Create( + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + string twoFactorAuthenticationCode) + { + Ensure.ArgumentNotNullOrEmptyString(clientId, "clientId"); + Ensure.ArgumentNotNullOrEmptyString(clientSecret, "clientSecret"); + Ensure.ArgumentNotNull(newAuthorization, "authorization"); + Ensure.ArgumentNotNullOrEmptyString(twoFactorAuthenticationCode, "twoFactorAuthenticationCode"); + + var requestData = new + { + client_id = clientId, + client_secret = clientSecret, + scopes = newAuthorization.Scopes, + note = newAuthorization.Note, + note_url = newAuthorization.NoteUrl, + fingerprint = newAuthorization.Fingerprint + }; + + var endpoint = ApiUrls.Authorizations(); + + return ApiConnection.Put(endpoint, requestData, twoFactorAuthenticationCode); + } + /// /// Creates a new authorization for the specified OAuth application if an authorization for that application doesn’t already /// exist for the user; otherwise, returns the user’s existing authorization for that application. diff --git a/Octokit/Clients/IAuthorizationsClient.cs b/Octokit/Clients/IAuthorizationsClient.cs index 9d050d54..8fb80bae 100644 --- a/Octokit/Clients/IAuthorizationsClient.cs +++ b/Octokit/Clients/IAuthorizationsClient.cs @@ -49,6 +49,56 @@ namespace Octokit Justification = "It's fiiiine. It's fine. Trust us.")] Task Get(int id); + /// + /// Creates a new authorization for the specified OAuth application if an authorization for that application + /// doesn’t already exist for the user; otherwise, it fails. + /// + /// + /// This method requires authentication. + /// See the API documentation for more information. + /// + /// Client ID of the OAuth application for the token + /// The client secret + /// Describes the new authorization to create + /// + /// Thrown when the current user does not have permission to make this request. + /// + /// + /// Thrown when the current account has two-factor authentication enabled and an authentication code is required. + /// + /// Thrown when a general API error occurs. + /// The created . + Task Create( + string clientId, + string clientSecret, + NewAuthorization newAuthorization); + + /// + /// Creates a new authorization for the specified OAuth application if an authorization for that application + /// doesn’t already exist for the user; otherwise, it fails. + /// + /// + /// This method requires authentication. + /// See the API documentation for more information. + /// + /// Client ID of the OAuth application for the token + /// The client secret + /// The two-factor authentication code in response to the current user's previous challenge + /// Describes the new authorization to create + /// + /// Thrown when the current user does not have permission to make this request. + /// + /// + /// Thrown when the current account has two-factor authentication enabled and an authentication code is required. + /// + /// Thrown when a general API error occurs. + /// The created . + Task Create( + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + string twoFactorAuthenticationCode); + /// /// Creates a new authorization for the specified OAuth application if an authorization for that application doesn’t already /// exist for the user; otherwise, returns the user’s existing authorization for that application. diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index a4c88909..4fcb69c2 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -376,6 +376,7 @@ namespace Octokit HttpMethod.Delete, null, null, + null, CancellationToken.None, twoFactorAuthenticationCode); return response.HttpResponse.StatusCode; From 2e1ade43f1b22369b1c4b499fe7792e0984072a8 Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 14:46:16 -0700 Subject: [PATCH 08/54] Implement extension method for authentication flow --- .../Helpers/AuthorizationExtensions.cs | 140 +++++++++++++++++- 1 file changed, 138 insertions(+), 2 deletions(-) diff --git a/Octokit.Reactive/Helpers/AuthorizationExtensions.cs b/Octokit.Reactive/Helpers/AuthorizationExtensions.cs index 36a110eb..2f918158 100644 --- a/Octokit.Reactive/Helpers/AuthorizationExtensions.cs +++ b/Octokit.Reactive/Helpers/AuthorizationExtensions.cs @@ -33,8 +33,7 @@ namespace Octokit string clientId, string clientSecret, NewAuthorization newAuthorization, - Func> twoFactorChallengeHandler - ) + Func> twoFactorChallengeHandler) { Ensure.ArgumentNotNull(authorizationsClient, "authorizationsClient"); Ensure.ArgumentNotNullOrEmptyString(clientId, "clientId"); @@ -55,5 +54,142 @@ namespace Octokit newAuthorization, result.AuthenticationCode))); } + + /// + /// This method will create a new authorization for the specified OAuth application. If an authorization + /// for that application already exists for the user and fingerprint, it'll delete the existing one and + /// recreate it. + /// + /// + /// + /// This method is typically used to initiate an application authentication flow. + /// This method allows the caller to provide a callback which is used to retrieve the two-factor code from + /// the user. Typically the callback is used to show some user interface to the user. + /// + /// + /// See API documentation + /// for more details. + /// + /// + /// The this method extends + /// Client ID for the OAuth application that is requesting the token + /// The client secret + /// Defines the scopes and metadata for the token + /// Callback used to retrieve the two-factor authentication code + /// from the user + /// If true, instead of completing when the two factor code supplied + /// is invalid, we go through the whole cycle again and prompt the two factor dialog. + /// + public static IObservable CreateAndDeleteExistingApplicationAuthentication( + this IObservableAuthorizationsClient authorizationsClient, + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + Func> twoFactorChallengeHandler, + bool retryInvalidTwoFactorCode) + { + return authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( + clientId, + clientSecret, + newAuthorization, + twoFactorChallengeHandler, + null, + retryInvalidTwoFactorCode); + } + + static IObservable CreateAndDeleteExistingApplicationAuthentication( + this IObservableAuthorizationsClient authorizationsClient, + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + Func> twoFactorChallengeHandler, + string twoFactorAuthenticationCode, + bool retryInvalidTwoFactorCode) + { + Ensure.ArgumentNotNull(authorizationsClient, "authorizationsClient"); + Ensure.ArgumentNotNullOrEmptyString(clientId, "clientId"); + Ensure.ArgumentNotNullOrEmptyString(clientSecret, "clientSecret"); + Ensure.ArgumentNotNull(newAuthorization, "newAuthorization"); + + return authorizationsClient.GetOrCreateAuthorizationUnified(clientId, clientSecret, newAuthorization, twoFactorAuthenticationCode) + .Catch(exception => twoFactorChallengeHandler(exception) + .SelectMany(result => + result.ResendCodeRequested + ? authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( + clientId, + clientSecret, + newAuthorization, + twoFactorChallengeHandler, + null, + retryInvalidTwoFactorCode) + : retryInvalidTwoFactorCode + // Attempt to create authorization and if it fails, show the 2fa dialog. + ? authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( + clientId, + clientSecret, + newAuthorization, + twoFactorChallengeHandler, + result.AuthenticationCode, + true) + // Attempt to create authorization and if it fails, don't show the 2fa dialog. + : authorizationsClient.CreateAuthorizationAndDeleteExisting( + clientId, + clientSecret, + newAuthorization, + result.AuthenticationCode))); + } + + // If the Application Authorization already exists, the result might have an empty string as the token. This is + // because GitHub.com no longer stores the token, but stores a hashed version of it. It is assumed that clients + // will store the token locally. + // The only reason to be calling GetOrCreateApplicationAuthentication is pretty much the case when you are + // logging in and thus don't have the token already. So if the token returned is an empty string, we'll go + // ahead and delete it for you and then recreate it. + static IObservable CreateAuthorizationAndDeleteExisting( + this IObservableAuthorizationsClient authorizationsClient, + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + string twoFactorAuthenticationCode = null) + { + return authorizationsClient.GetOrCreateAuthorizationUnified( + clientId, + clientSecret, + newAuthorization, + twoFactorAuthenticationCode) + .SelectMany(authorization => string.IsNullOrEmpty(authorization.Token) + ? authorizationsClient.Delete(authorization.Id, twoFactorAuthenticationCode) + .SelectMany(_ => + authorizationsClient.CreateNewAuthorization( + clientId, + clientSecret, + newAuthorization, + twoFactorAuthenticationCode)) + : Observable.Return(authorization)); + } + + static IObservable GetOrCreateAuthorizationUnified( + this IObservableAuthorizationsClient authorizationsClient, + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + string twoFactorAuthenticationCode = null) + { + return string.IsNullOrEmpty(twoFactorAuthenticationCode) + ? authorizationsClient.GetOrCreateApplicationAuthentication(clientId, clientSecret, newAuthorization) + : authorizationsClient.GetOrCreateApplicationAuthentication(clientId, clientSecret, newAuthorization, twoFactorAuthenticationCode); + } + + static IObservable CreateNewAuthorization( + this IObservableAuthorizationsClient authorizationsClient, + string clientId, + string clientSecret, + NewAuthorization newAuthorization, + string twoFactorAuthenticationCode = null) + { + return string.IsNullOrEmpty(twoFactorAuthenticationCode) + ? authorizationsClient.Create(clientId, clientSecret, newAuthorization) + : authorizationsClient.Create(clientId, clientSecret, newAuthorization, twoFactorAuthenticationCode); + } } } From c401d7e2837ff1636e1eff766ba0f69bdcf441b1 Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 15:03:12 -0700 Subject: [PATCH 09/54] Fix extension method to handle GHfW and other scenarios In GHfW, if you submit an authentication code, and it's wrong, we go back to the login dialog. In other applications, you may want to retry the 2fa dialog and show the error there. --- .../Helpers/AuthorizationExtensions.cs | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/Octokit.Reactive/Helpers/AuthorizationExtensions.cs b/Octokit.Reactive/Helpers/AuthorizationExtensions.cs index 2f918158..c283408e 100644 --- a/Octokit.Reactive/Helpers/AuthorizationExtensions.cs +++ b/Octokit.Reactive/Helpers/AuthorizationExtensions.cs @@ -85,7 +85,7 @@ namespace Octokit string clientId, string clientSecret, NewAuthorization newAuthorization, - Func> twoFactorChallengeHandler, + Func> twoFactorChallengeHandler, bool retryInvalidTwoFactorCode) { return authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( @@ -102,7 +102,7 @@ namespace Octokit string clientId, string clientSecret, NewAuthorization newAuthorization, - Func> twoFactorChallengeHandler, + Func> twoFactorChallengeHandler, string twoFactorAuthenticationCode, bool retryInvalidTwoFactorCode) { @@ -111,32 +111,37 @@ namespace Octokit Ensure.ArgumentNotNullOrEmptyString(clientSecret, "clientSecret"); Ensure.ArgumentNotNull(newAuthorization, "newAuthorization"); - return authorizationsClient.GetOrCreateAuthorizationUnified(clientId, clientSecret, newAuthorization, twoFactorAuthenticationCode) - .Catch(exception => twoFactorChallengeHandler(exception) + // If retryInvalidTwoFactorCode is false, then we only show the TwoFactorDialog when we catch + // a TwoFactorRequiredException. If it's true, we show it for TwoFactorRequiredException and + // TwoFactorChallengeFailedException + Func> twoFactorHandler = ex => + retryInvalidTwoFactorCode || ex is TwoFactorRequiredException + ? twoFactorChallengeHandler(ex) + : Observable.Throw(ex); + + return authorizationsClient.CreateAuthorizationAndDeleteExisting( + clientId, + clientSecret, + newAuthorization, + twoFactorAuthenticationCode) + .Catch( + exception => twoFactorHandler(exception) .SelectMany(result => result.ResendCodeRequested ? authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( clientId, clientSecret, newAuthorization, - twoFactorChallengeHandler, - null, + twoFactorHandler, + null, // twoFactorAuthenticationCode retryInvalidTwoFactorCode) - : retryInvalidTwoFactorCode - // Attempt to create authorization and if it fails, show the 2fa dialog. - ? authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( + : authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( clientId, clientSecret, newAuthorization, - twoFactorChallengeHandler, + twoFactorHandler, result.AuthenticationCode, - true) - // Attempt to create authorization and if it fails, don't show the 2fa dialog. - : authorizationsClient.CreateAuthorizationAndDeleteExisting( - clientId, - clientSecret, - newAuthorization, - result.AuthenticationCode))); + retryInvalidTwoFactorCode))); } // If the Application Authorization already exists, the result might have an empty string as the token. This is From 687783c10d21af5f805d1206c9dd2cf05e822aea Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 07:47:57 +0930 Subject: [PATCH 10/54] fix impacted unit tests --- Octokit.Tests/Clients/AuthorizationsClientTests.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Octokit.Tests/Clients/AuthorizationsClientTests.cs b/Octokit.Tests/Clients/AuthorizationsClientTests.cs index 05646ef3..8894abaf 100644 --- a/Octokit.Tests/Clients/AuthorizationsClientTests.cs +++ b/Octokit.Tests/Clients/AuthorizationsClientTests.cs @@ -36,8 +36,7 @@ namespace Octokit.Tests.Clients client.Received().GetAll( Arg.Is(u => u.ToString() == "authorizations"), - null, - Arg.Any()); + null); } } @@ -53,8 +52,7 @@ namespace Octokit.Tests.Clients client.Received().Get( Arg.Is(u => u.ToString() == "authorizations/1"), - null, - Arg.Any()); + null); } } @@ -238,9 +236,7 @@ namespace Octokit.Tests.Clients authEndpoint.GetOrCreateApplicationAuthentication("clientId", "secret", data); client.Received().Put(Arg.Is(u => u.ToString() == "authorizations/clients/clientId/ha-ha-fingerprint"), - Args.Object, - Args.String, - Args.String); // NOTE: preview API + Args.Object); } } @@ -256,8 +252,7 @@ namespace Octokit.Tests.Clients client.Received().Get( Arg.Is(u => u.ToString() == "applications/clientId/tokens/accessToken"), - null, - Arg.Any()); + null); } [Fact] From 80c4dc9d18a15f04e7c3ae151b2c9aae756b297b Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 15:32:48 -0700 Subject: [PATCH 11/54] Kinda helps when you pass in the argument --- Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs b/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs index 7192412d..2a4d1961 100644 --- a/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs +++ b/Octokit.Reactive/Clients/ObservableAuthorizationsClient.cs @@ -107,7 +107,7 @@ namespace Octokit.Reactive Ensure.ArgumentNotNull(newAuthorization, "authorization"); Ensure.ArgumentNotNullOrEmptyString(twoFactorAuthenticationCode, "twoFactorAuthenticationCode"); - return _client.Create(clientId, clientSecret, newAuthorization).ToObservable(); + return _client.Create(clientId, clientSecret, newAuthorization, twoFactorAuthenticationCode).ToObservable(); } /// From 79446b68cc5690411fd89af1df0f5990ad1c7bd1 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 08:01:12 +0930 Subject: [PATCH 12/54] updated integration test --- .../Clients/AuthorizationClientTests.cs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/AuthorizationClientTests.cs b/Octokit.Tests.Integration/Clients/AuthorizationClientTests.cs index 5872af17..710a4c9a 100644 --- a/Octokit.Tests.Integration/Clients/AuthorizationClientTests.cs +++ b/Octokit.Tests.Integration/Clients/AuthorizationClientTests.cs @@ -22,10 +22,9 @@ namespace Octokit.Tests.Integration.Clients Helper.ClientSecret, newAuthorization); - Assert.NotNull(created); Assert.False(String.IsNullOrWhiteSpace(created.Token)); - Assert.True(String.IsNullOrWhiteSpace(created.TokenLastEight)); - Assert.True(String.IsNullOrWhiteSpace(created.HashedToken)); + Assert.False(String.IsNullOrWhiteSpace(created.TokenLastEight)); + Assert.False(String.IsNullOrWhiteSpace(created.HashedToken)); // we can then query it through the regular API var get = await client.Authorization.Get(created.Id); @@ -42,13 +41,11 @@ namespace Octokit.Tests.Integration.Clients Assert.Equal(created.Id, getExisting.Id); - // NOTE: the old API will continue to return the full - // token if no Fingerprint is included - Assert.False(String.IsNullOrWhiteSpace(getExisting.Token)); - - // NOTE: and these new values are not included - Assert.True(String.IsNullOrWhiteSpace(getExisting.TokenLastEight)); - Assert.True(String.IsNullOrWhiteSpace(getExisting.HashedToken)); + // the token is no longer returned for subsequent calls + Assert.True(String.IsNullOrWhiteSpace(getExisting.Token)); + // however the hashed and last 8 characters are available + Assert.False(String.IsNullOrWhiteSpace(getExisting.TokenLastEight)); + Assert.False(String.IsNullOrWhiteSpace(getExisting.HashedToken)); await client.Authorization.Delete(created.Id); } From 7f7dee4097d3b6f2dfeccd222122c26b0497918c Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 15:38:46 -0700 Subject: [PATCH 13/54] Someday I'll learn how HTTP works --- Octokit/Clients/AuthorizationsClient.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 73fd742b..614df00e 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -143,7 +143,7 @@ namespace Octokit var endpoint = ApiUrls.Authorizations(); - return ApiConnection.Put(endpoint, requestData, twoFactorAuthenticationCode); + return ApiConnection.Post(endpoint, requestData, twoFactorAuthenticationCode); } /// @@ -186,7 +186,7 @@ namespace Octokit ? ApiUrls.AuthorizationsForClient(clientId) : ApiUrls.AuthorizationsForClient(clientId, newAuthorization.Fingerprint); - return ApiConnection.Put(endpoint, requestData); + return ApiConnection.Post(endpoint, requestData); } /// From d27922f5ca60eef1d825887f068ddf785b01717f Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 08:14:56 +0930 Subject: [PATCH 14/54] that haacked guy --- Octokit.Tests/Clients/AuthorizationsClientTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Octokit.Tests/Clients/AuthorizationsClientTests.cs b/Octokit.Tests/Clients/AuthorizationsClientTests.cs index 8894abaf..7d625da0 100644 --- a/Octokit.Tests/Clients/AuthorizationsClientTests.cs +++ b/Octokit.Tests/Clients/AuthorizationsClientTests.cs @@ -96,7 +96,7 @@ namespace Octokit.Tests.Clients authEndpoint.GetOrCreateApplicationAuthentication("clientId", "secret", data); - client.Received().Put(Arg.Is(u => u.ToString() == "authorizations/clients/clientId"), + client.Received().Post(Arg.Is(u => u.ToString() == "authorizations/clients/clientId"), Args.Object); } @@ -235,7 +235,7 @@ namespace Octokit.Tests.Clients authEndpoint.GetOrCreateApplicationAuthentication("clientId", "secret", data); - client.Received().Put(Arg.Is(u => u.ToString() == "authorizations/clients/clientId/ha-ha-fingerprint"), + client.Received().Post(Arg.Is(u => u.ToString() == "authorizations/clients/clientId/ha-ha-fingerprint"), Args.Object); } } From afd5f8ab569750600d1ed4599cc2d4b7540d1b2b Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 16:12:22 -0700 Subject: [PATCH 15/54] Help POST learn about 2fa codes --- Octokit/Clients/AuthorizationsClient.cs | 3 +-- Octokit/Http/ApiConnection.cs | 26 +++++++++++++++++++++++++ Octokit/Http/Connection.cs | 21 ++++++++++++++++++++ Octokit/Http/IApiConnection.cs | 13 +++++++++++++ Octokit/Http/IConnection.cs | 13 +++++++++++++ 5 files changed, 74 insertions(+), 2 deletions(-) diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 614df00e..dad1bd8b 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -142,8 +142,7 @@ namespace Octokit }; var endpoint = ApiUrls.Authorizations(); - - return ApiConnection.Post(endpoint, requestData, twoFactorAuthenticationCode); + return ApiConnection.Post(endpoint, requestData, null, null, twoFactorAuthenticationCode); } /// diff --git a/Octokit/Http/ApiConnection.cs b/Octokit/Http/ApiConnection.cs index 51113271..b97b1afe 100644 --- a/Octokit/Http/ApiConnection.cs +++ b/Octokit/Http/ApiConnection.cs @@ -186,6 +186,32 @@ namespace Octokit return response.Body; } + /// + /// Creates a new API resource in the list at the specified URI. + /// + /// The API resource's type. + /// URI of the API resource to get + /// Object that describes the new API resource; this will be serialized and used as the request's body + /// Accept header to use for the API request + /// Content type of the API request + /// Two Factor Authentication Code + /// The created API resource. + /// Thrown when an API error occurs. + public async Task Post(Uri uri, object data, string accepts, string contentType, string twoFactorAuthenticationCode) + { + Ensure.ArgumentNotNull(uri, "uri"); + Ensure.ArgumentNotNull(data, "data"); + Ensure.ArgumentNotNull(twoFactorAuthenticationCode, "twoFactorAuthenticationCode"); + + var response = await Connection.Post( + uri, + data, + accepts, + contentType).ConfigureAwait(false); + return response.Body; + } + + public async Task Post(Uri uri, object data, string accepts, string contentType, TimeSpan timeout) { Ensure.ArgumentNotNull(uri, "uri"); diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 4fcb69c2..7d2f3e93 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -192,6 +192,27 @@ namespace Octokit return SendData(uri, HttpMethod.Post, body, accepts, contentType, CancellationToken.None); } + /// + /// Performs an asynchronous HTTP POST request. + /// Attempts to map the response body to an object of type + /// + /// The type to map the response to + /// URI endpoint to send request to + /// The object to serialize as the body of the request + /// Specifies accepted response media types. + /// Specifies the media type of the request body + /// Two Factor Authentication Code + /// representing the received HTTP response + public Task> Post(Uri uri, object body, string accepts, string contentType, string twoFactorAuthenticationCode) + { + Ensure.ArgumentNotNull(uri, "uri"); + Ensure.ArgumentNotNull(body, "body"); + Ensure.ArgumentNotNullOrEmptyString(twoFactorAuthenticationCode, "twoFactorAuthenticationCode"); + + return SendData(uri, HttpMethod.Post, body, accepts, contentType, CancellationToken.None, twoFactorAuthenticationCode); + + } + public Task> Post(Uri uri, object body, string accepts, string contentType, TimeSpan timeout) { Ensure.ArgumentNotNull(uri, "uri"); diff --git a/Octokit/Http/IApiConnection.cs b/Octokit/Http/IApiConnection.cs index 5bf11f8b..9c6fee79 100644 --- a/Octokit/Http/IApiConnection.cs +++ b/Octokit/Http/IApiConnection.cs @@ -114,6 +114,19 @@ namespace Octokit /// Thrown when an API error occurs. Task Post(Uri uri, object data, string accepts, string contentType); + /// + /// Creates a new API resource in the list at the specified URI. + /// + /// The API resource's type. + /// URI of the API resource to get + /// Object that describes the new API resource; this will be serialized and used as the request's body + /// Accept header to use for the API request + /// Content type of the API request + /// Two Factor Authentication Code + /// The created API resource. + /// Thrown when an API error occurs. + Task Post(Uri uri, object data, string accepts, string contentType, string twoFactorAuthenticationCode); + /// /// Creates a new API resource in the list at the specified URI. /// diff --git a/Octokit/Http/IConnection.cs b/Octokit/Http/IConnection.cs index 37a6c004..afc5b938 100644 --- a/Octokit/Http/IConnection.cs +++ b/Octokit/Http/IConnection.cs @@ -85,6 +85,19 @@ namespace Octokit /// representing the received HTTP response Task> Post(Uri uri, object body, string accepts, string contentType); + /// + /// Performs an asynchronous HTTP POST request. + /// Attempts to map the response body to an object of type + /// + /// The type to map the response to + /// URI endpoint to send request to + /// The object to serialize as the body of the request + /// Specifies accepted response media types. + /// Specifies the media type of the request body + /// Two Factor Authentication Code + /// representing the received HTTP response + Task> Post(Uri uri, object body, string accepts, string contentType, string twoFactorAuthenticationCode); + /// /// Performs an asynchronous HTTP POST request. /// Attempts to map the response body to an object of type From 119d331b442798df31c442fcdb0199885b096f41 Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 16:25:08 -0700 Subject: [PATCH 16/54] Somebody take the keyboard away from me --- Octokit/Clients/AuthorizationsClient.cs | 2 +- Octokit/Http/ApiConnection.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index dad1bd8b..9210d0c3 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -97,7 +97,7 @@ namespace Octokit var endpoint = ApiUrls.Authorizations(); - return ApiConnection.Put(endpoint, requestData); + return ApiConnection.Post(endpoint, requestData); } /// diff --git a/Octokit/Http/ApiConnection.cs b/Octokit/Http/ApiConnection.cs index b97b1afe..145ef8ab 100644 --- a/Octokit/Http/ApiConnection.cs +++ b/Octokit/Http/ApiConnection.cs @@ -207,7 +207,8 @@ namespace Octokit uri, data, accepts, - contentType).ConfigureAwait(false); + contentType, + twoFactorAuthenticationCode).ConfigureAwait(false); return response.Body; } From c107abffc45215c0a0d100246aa02c6045f20e6e Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 21 Apr 2015 17:22:56 -0700 Subject: [PATCH 17/54] this should be PUT --- Octokit/Clients/AuthorizationsClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 9210d0c3..716cfad1 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -185,7 +185,7 @@ namespace Octokit ? ApiUrls.AuthorizationsForClient(clientId) : ApiUrls.AuthorizationsForClient(clientId, newAuthorization.Fingerprint); - return ApiConnection.Post(endpoint, requestData); + return ApiConnection.Put(endpoint, requestData); } /// From e76c8b9f2d0a9b4df96a3f05f0c55b4ba0df6ddc Mon Sep 17 00:00:00 2001 From: Haacked Date: Tue, 21 Apr 2015 17:26:06 -0700 Subject: [PATCH 18/54] Make a method public And rename them for good cause. --- Octokit.Reactive/Helpers/AuthorizationExtensions.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Octokit.Reactive/Helpers/AuthorizationExtensions.cs b/Octokit.Reactive/Helpers/AuthorizationExtensions.cs index c283408e..536fcab5 100644 --- a/Octokit.Reactive/Helpers/AuthorizationExtensions.cs +++ b/Octokit.Reactive/Helpers/AuthorizationExtensions.cs @@ -80,7 +80,7 @@ namespace Octokit /// If true, instead of completing when the two factor code supplied /// is invalid, we go through the whole cycle again and prompt the two factor dialog. /// - public static IObservable CreateAndDeleteExistingApplicationAuthentication( + public static IObservable CreateAndDeleteExistingApplicationAuthorization( this IObservableAuthorizationsClient authorizationsClient, string clientId, string clientSecret, @@ -88,7 +88,7 @@ namespace Octokit Func> twoFactorChallengeHandler, bool retryInvalidTwoFactorCode) { - return authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( + return authorizationsClient.CreateAndDeleteExistingApplicationAuthorization( clientId, clientSecret, newAuthorization, @@ -97,7 +97,7 @@ namespace Octokit retryInvalidTwoFactorCode); } - static IObservable CreateAndDeleteExistingApplicationAuthentication( + public static IObservable CreateAndDeleteExistingApplicationAuthorization( this IObservableAuthorizationsClient authorizationsClient, string clientId, string clientSecret, @@ -128,14 +128,14 @@ namespace Octokit exception => twoFactorHandler(exception) .SelectMany(result => result.ResendCodeRequested - ? authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( + ? authorizationsClient.CreateAndDeleteExistingApplicationAuthorization( clientId, clientSecret, newAuthorization, twoFactorHandler, null, // twoFactorAuthenticationCode retryInvalidTwoFactorCode) - : authorizationsClient.CreateAndDeleteExistingApplicationAuthentication( + : authorizationsClient.CreateAndDeleteExistingApplicationAuthorization( clientId, clientSecret, newAuthorization, From a87d59b189dcb8e5a31c29bbb0a443aeb1013e89 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 10:52:01 +0930 Subject: [PATCH 19/54] started on docs --- ReleaseNotes.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ReleaseNotes.md b/ReleaseNotes.md index 9090c8da..e731cd2f 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -1,3 +1,11 @@ +### New in 0.10.0 (released 2015/04/22) +* Fixed: added `since` overload to `Repositories.GetAllPublic` - #774 via @alfhenrik +* Fixed: renamed methods to follow `GetAll` convention - #771 via @alfhenrik +* Fixed: helper functions and cleanup to make using Authorization API easier to consume - #786 via @haacked + +**Breaking Changes:** + - TODO + ### New in 0.9.0 (released 2015/04/04) * New: added `PullRequest.Files` APIs - #752 via @alfhenrik * Fixed: `PullRequestRequest` now supports `SortDirection` and `SortProperty` - #752 via @alfhenrik From 43ba17f25b19c62cac587bff470a112ea704cf6c Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 10:54:25 +0930 Subject: [PATCH 20/54] version bump --- SolutionInfo.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SolutionInfo.cs b/SolutionInfo.cs index 1798e252..78217389 100644 --- a/SolutionInfo.cs +++ b/SolutionInfo.cs @@ -3,11 +3,11 @@ using System.Reflection; using System.Runtime.InteropServices; [assembly: AssemblyProductAttribute("Octokit")] -[assembly: AssemblyVersionAttribute("0.9.0")] -[assembly: AssemblyFileVersionAttribute("0.9.0")] +[assembly: AssemblyVersionAttribute("0.10.0")] +[assembly: AssemblyFileVersionAttribute("0.10.0")] [assembly: ComVisibleAttribute(false)] namespace System { internal static class AssemblyVersionInformation { - internal const string Version = "0.9.0"; + internal const string Version = "0.10.0"; } } From f280fb3830f5ad0db2933d033d1ca2e72c510ead Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 11:20:08 +0930 Subject: [PATCH 21/54] fixed tests --- Octokit.Tests/Clients/AuthorizationsClientTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Octokit.Tests/Clients/AuthorizationsClientTests.cs b/Octokit.Tests/Clients/AuthorizationsClientTests.cs index 7d625da0..8894abaf 100644 --- a/Octokit.Tests/Clients/AuthorizationsClientTests.cs +++ b/Octokit.Tests/Clients/AuthorizationsClientTests.cs @@ -96,7 +96,7 @@ namespace Octokit.Tests.Clients authEndpoint.GetOrCreateApplicationAuthentication("clientId", "secret", data); - client.Received().Post(Arg.Is(u => u.ToString() == "authorizations/clients/clientId"), + client.Received().Put(Arg.Is(u => u.ToString() == "authorizations/clients/clientId"), Args.Object); } @@ -235,7 +235,7 @@ namespace Octokit.Tests.Clients authEndpoint.GetOrCreateApplicationAuthentication("clientId", "secret", data); - client.Received().Post(Arg.Is(u => u.ToString() == "authorizations/clients/clientId/ha-ha-fingerprint"), + client.Received().Put(Arg.Is(u => u.ToString() == "authorizations/clients/clientId/ha-ha-fingerprint"), Args.Object); } } From bd1d98340d7bba88e991bba334301d40f428dd50 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 11:20:22 +0930 Subject: [PATCH 22/54] breaking changes notes --- ReleaseNotes.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ReleaseNotes.md b/ReleaseNotes.md index e731cd2f..da10d384 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -4,7 +4,10 @@ * Fixed: helper functions and cleanup to make using Authorization API easier to consume - #786 via @haacked **Breaking Changes:** - - TODO + - As part of #771 there were many method which were returning collections + but the method name made it unclear. As part of enforcing this convention, + if you have a method that doesn't compile, it is likely that you need to + set the prefix to `GetAll` to re-find that API. ### New in 0.9.0 (released 2015/04/04) * New: added `PullRequest.Files` APIs - #752 via @alfhenrik From 92045a58c98468915b1095d30119741299efde5a Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 11:57:03 +0930 Subject: [PATCH 23/54] typos in release notes --- ReleaseNotes.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ReleaseNotes.md b/ReleaseNotes.md index da10d384..bab3039e 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -5,9 +5,9 @@ **Breaking Changes:** - As part of #771 there were many method which were returning collections - but the method name made it unclear. As part of enforcing this convention, - if you have a method that doesn't compile, it is likely that you need to - set the prefix to `GetAll` to re-find that API. + but the method name made it unclear. You might think that it wasn't much, but + you'd be wrong. So if you have a method that no longer compile, + it is likely that you need to set the prefix to `GetAll` to re-disocver that API. ### New in 0.9.0 (released 2015/04/04) * New: added `PullRequest.Files` APIs - #752 via @alfhenrik From 37f51fe7deeab9dceb5ff81adf77c4ec6e59a031 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 11:59:34 +0930 Subject: [PATCH 24/54] commit comments may not be tied to a line number --- Octokit/Models/Response/CommitComment.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit/Models/Response/CommitComment.cs b/Octokit/Models/Response/CommitComment.cs index 9685cc0e..f2db8e87 100644 --- a/Octokit/Models/Response/CommitComment.cs +++ b/Octokit/Models/Response/CommitComment.cs @@ -56,7 +56,7 @@ namespace Octokit /// /// Line index in the diff that was commented on. /// - public int Position { get; protected set; } + public int? Position { get; protected set; } /// /// The line number in the file that was commented on. From ca5dc4a79ca6ff528e0756698e36fbc75a7536fc Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 11:59:41 +0930 Subject: [PATCH 25/54] muting some broken tests --- Octokit.Tests.Integration/Clients/EventsClientTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/EventsClientTests.cs b/Octokit.Tests.Integration/Clients/EventsClientTests.cs index 666d4104..2802bbff 100644 --- a/Octokit.Tests.Integration/Clients/EventsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/EventsClientTests.cs @@ -34,7 +34,7 @@ namespace Octokit.Tests.Integration.Clients Assert.True(_events.All(e => e.Payload != null)); } - [IntegrationTest] + [IntegrationTest(Skip = "no longer able to access this event")] public void IssueCommentPayloadEventDeserializesCorrectly() { var commentEvent = _events.FirstOrDefault(e => e.Id == "2628548686"); @@ -49,7 +49,7 @@ namespace Octokit.Tests.Integration.Clients Assert.Equal(742, commentPayload.Issue.Number); } - [IntegrationTest] + [IntegrationTest(Skip = "no longer able to access this event")] public void PushEventPayloadDeserializesCorrectly() { var pushEvent = _events.FirstOrDefault(e => e.Id == "2628858765"); @@ -65,7 +65,7 @@ namespace Octokit.Tests.Integration.Clients Assert.Equal(1, pushPayload.Size); } - [IntegrationTest] + [IntegrationTest(Skip = "no longer able to access this event")] public void PREventPayloadDeserializesCorrectly() { var prEvent = _events.FirstOrDefault(e => e.Id == "2628718313"); @@ -79,7 +79,7 @@ namespace Octokit.Tests.Integration.Clients Assert.Equal(743, prPayload.PullRequest.Number); } - [IntegrationTest] + [IntegrationTest(Skip = "no longer able to access this event")] public void PRReviewCommentEventPayloadDeserializesCorrectly() { var prrcEvent = _events.First(e => e.Id == "2623246246"); From 082d3ed03b866959544ec20a6a676b959c94250d Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 12:01:51 +0930 Subject: [PATCH 26/54] more notes --- ReleaseNotes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ReleaseNotes.md b/ReleaseNotes.md index bab3039e..68686730 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -8,6 +8,7 @@ but the method name made it unclear. You might think that it wasn't much, but you'd be wrong. So if you have a method that no longer compile, it is likely that you need to set the prefix to `GetAll` to re-disocver that API. + - `CommitComment.Position` is now a nullable `int` to prevent serialization issues. ### New in 0.9.0 (released 2015/04/04) * New: added `PullRequest.Files` APIs - #752 via @alfhenrik From 26a8bf0e818ad8a620987e13d5ed7b1b8bacfa01 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 12:18:00 +0930 Subject: [PATCH 27/54] removed GetAllPublic overload due to paging issue and parameters clobbering --- .../Clients/IObservableRepositoriesClient.cs | 12 ---- .../Clients/ObservableRepositoriesClient.cs | 15 ----- .../Clients/RepositoriesClientTests.cs | 24 ++++---- .../ObservableRepositoriesClientTests.cs | 17 ------ .../Clients/RepositoriesClientTests.cs | 31 ---------- .../ObservableRepositoriesClientTests.cs | 59 ------------------- Octokit/Clients/IRepositoriesClient.cs | 14 ----- Octokit/Clients/RepositoriesClient.cs | 18 ------ 8 files changed, 12 insertions(+), 178 deletions(-) diff --git a/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs b/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs index 25337bab..9460a197 100644 --- a/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs +++ b/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs @@ -50,18 +50,6 @@ namespace Octokit.Reactive Justification = "Makes a network request")] IObservable GetAllPublic(); - /// - /// Retrieves every public since the last repository seen. - /// - /// - /// The default page size on GitHub.com is 30. - /// - /// Search parameters of the last repository seen - /// A of . - [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate", - Justification = "Makes a network request")] - IObservable GetAllPublic(PublicRepositoryRequest request); - /// /// Retrieves every that belongs to the current user. /// diff --git a/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs b/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs index 1a173d9c..9c17dbf8 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs @@ -102,21 +102,6 @@ namespace Octokit.Reactive return _connection.GetAndFlattenAllPages(ApiUrls.AllPublicRepositories()); } - /// - /// Retrieves every public since the last repository seen. - /// - /// - /// The default page size on GitHub.com is 30. - /// - /// Search parameters of the last repository seen - /// A of . - public IObservable GetAllPublic(PublicRepositoryRequest request) - { - Ensure.ArgumentNotNull(request, "request"); - - return _connection.GetAndFlattenAllPages(ApiUrls.AllPublicRepositories(), request.ToParametersDictionary()); - } - /// /// Retrieves every that belongs to the current user. /// diff --git a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs index 6c40f537..b6c935e2 100644 --- a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs @@ -553,20 +553,20 @@ public class RepositoriesClientTests Assert.True(repositories.Count > 80); } - [IntegrationTest] - public async Task ReturnsAllPublicRepositoriesSinceLastSeen() - { - var github = Helper.GetAuthenticatedClient(); + //[IntegrationTest] + //public async Task ReturnsAllPublicRepositoriesSinceLastSeen() + //{ + // var github = Helper.GetAuthenticatedClient(); - var request = new PublicRepositoryRequest(32732250); - var repositories = await github.Repository.GetAllPublic(request); + // var request = new PublicRepositoryRequest(32732250); + // var repositories = await github.Repository.GetAllPublic(request); - Assert.NotNull(repositories); - Assert.True(repositories.Any()); - Assert.Equal(32732252, repositories[0].Id); - Assert.False(repositories[0].Private); - Assert.Equal("zad19", repositories[0].Name); - } + // Assert.NotNull(repositories); + // Assert.True(repositories.Any()); + // Assert.Equal(32732252, repositories[0].Id); + // Assert.False(repositories[0].Private); + // Assert.Equal("zad19", repositories[0].Name); + //} } public class TheGetAllForOrgMethod diff --git a/Octokit.Tests.Integration/Reactive/ObservableRepositoriesClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableRepositoriesClientTests.cs index 397eb663..68341f58 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableRepositoriesClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableRepositoriesClientTests.cs @@ -28,22 +28,5 @@ namespace Octokit.Tests.Integration Assert.False(repository2.Fork); } } - - public class TheGetAllPublicSinceMethod - { - [IntegrationTest(Skip = "This will take a very long time to return, so will skip it for now.")] - public async Task ReturnsAllPublicReposSinceLastSeen() - { - var github = Helper.GetAuthenticatedClient(); - - var client = new ObservableRepositoriesClient(github); - var request = new PublicRepositoryRequest(32732250); - var repositories = await client.GetAllPublic(request).ToArray(); - Assert.NotEmpty(repositories); - Assert.Equal(32732252, repositories[0].Id); - Assert.False(repositories[0].Private); - Assert.Equal("zad19", repositories[0].Name); - } - } } } diff --git a/Octokit.Tests/Clients/RepositoriesClientTests.cs b/Octokit.Tests/Clients/RepositoriesClientTests.cs index ed73c0f7..0534472b 100644 --- a/Octokit.Tests/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoriesClientTests.cs @@ -273,37 +273,6 @@ namespace Octokit.Tests.Clients } } - - public class TheGetAllPublicSinceMethod - { - [Fact] - public void RequestsTheCorrectUrl() - { - var connection = Substitute.For(); - var client = new RepositoriesClient(connection); - - client.GetAllPublic(new PublicRepositoryRequest(364)); - - connection.Received() - .GetAll(Arg.Is(u => u.ToString() == "/repositories"), - Arg.Any>()); - } - - [Fact] - public void SendsTheCorrectParameter() - { - var connection = Substitute.For(); - var client = new RepositoriesClient(connection); - - client.GetAllPublic(new PublicRepositoryRequest(364)); - - connection.Received() - .GetAll(Arg.Is(u => u.ToString() == "/repositories"), - Arg.Is>(d => d.Count == 1 - && d["since"] == "364")); - } - } - public class TheGetAllForCurrentMethod { [Fact] diff --git a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs index d1668b14..044a74d7 100644 --- a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs @@ -158,65 +158,6 @@ namespace Octokit.Tests.Reactive } } - public class TheGetAllPublicRepositoriesSinceMethod - { - [Fact] - public async Task ReturnsEveryPageOfRepositories() - { - var firstPageUrl = new Uri("/repositories", UriKind.Relative); - var secondPageUrl = new Uri("https://example.com/page/2"); - var firstPageLinks = new Dictionary { { "next", secondPageUrl } }; - IApiResponse> firstPageResponse = new ApiResponse>( - CreateResponseWithApiInfo(firstPageLinks), - new List - { - new Repository(364), - new Repository(365), - new Repository(366) - }); - - var thirdPageUrl = new Uri("https://example.com/page/3"); - var secondPageLinks = new Dictionary { { "next", thirdPageUrl } }; - IApiResponse> secondPageResponse = new ApiResponse> - ( - CreateResponseWithApiInfo(secondPageLinks), - new List - { - new Repository(367), - new Repository(368), - new Repository(369) - }); - - IApiResponse> lastPageResponse = new ApiResponse>( - new Response(), - new List - { - new Repository(370) - }); - - var gitHubClient = Substitute.For(); - gitHubClient.Connection.Get>(firstPageUrl, - Arg.Is>(d => d.Count == 1 - && d["since"] == "364"), null) - .Returns(Task.FromResult(firstPageResponse)); - gitHubClient.Connection.Get>(secondPageUrl, null, null) - .Returns(Task.FromResult(secondPageResponse)); - gitHubClient.Connection.Get>(thirdPageUrl, null, null) - .Returns(Task.FromResult(lastPageResponse)); - - var repositoriesClient = new ObservableRepositoriesClient(gitHubClient); - - var results = await repositoriesClient.GetAllPublic(new PublicRepositoryRequest(364)).ToArray(); - - Assert.Equal(7, results.Length); - gitHubClient.Connection.Received(1).Get>(firstPageUrl, - Arg.Is>(d=>d.Count == 1 - && d["since"] == "364"), null); - gitHubClient.Connection.Received(1).Get>(secondPageUrl, null, null); - gitHubClient.Connection.Received(1).Get>(thirdPageUrl, null, null); - } - } - public class TheGetAllBranchesMethod { [Fact] diff --git a/Octokit/Clients/IRepositoriesClient.cs b/Octokit/Clients/IRepositoriesClient.cs index e6f07092..7172f0f4 100644 --- a/Octokit/Clients/IRepositoriesClient.cs +++ b/Octokit/Clients/IRepositoriesClient.cs @@ -109,20 +109,6 @@ namespace Octokit Justification = "Makes a network request")] Task> GetAllPublic(); - - /// - /// Gets all public repositories since the integer ID of the last Repository that you've seen. - /// - /// - /// See the API documentation for more information. - /// The default page size on GitHub.com is 30. - /// - /// Search parameters of the last repository seen - /// Thrown if the client is not authenticated. - /// Thrown when a general API error occurs. - /// A of . - Task> GetAllPublic(PublicRepositoryRequest request); - /// /// Gets all repositories owned by the current user. /// diff --git a/Octokit/Clients/RepositoriesClient.cs b/Octokit/Clients/RepositoriesClient.cs index 479ac89f..5de9ceec 100644 --- a/Octokit/Clients/RepositoriesClient.cs +++ b/Octokit/Clients/RepositoriesClient.cs @@ -188,24 +188,6 @@ namespace Octokit return ApiConnection.GetAll(ApiUrls.AllPublicRepositories()); } - /// - /// Gets all public repositories since the integer ID of the last Repository that you've seen. - /// - /// - /// See the API documentation for more information. - /// The default page size on GitHub.com is 30. - /// - /// Search parameters of the last repository seen - /// Thrown if the client is not authenticated. - /// Thrown when a general API error occurs. - /// A of . - public Task> GetAllPublic(PublicRepositoryRequest request) - { - Ensure.ArgumentNotNull(request, "request"); - - return ApiConnection.GetAll(ApiUrls.AllPublicRepositories(), request.ToParametersDictionary()); - } - /// /// Gets all repositories owned by the current user. /// From ad268b26149fb44e597f3b97f312645064a3ed9c Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Wed, 22 Apr 2015 12:19:56 +0930 Subject: [PATCH 28/54] sorry alfhenrik, i stuffed up this review big time --- ReleaseNotes.md | 1 - 1 file changed, 1 deletion(-) diff --git a/ReleaseNotes.md b/ReleaseNotes.md index 68686730..e9d39de7 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -1,5 +1,4 @@ ### New in 0.10.0 (released 2015/04/22) -* Fixed: added `since` overload to `Repositories.GetAllPublic` - #774 via @alfhenrik * Fixed: renamed methods to follow `GetAll` convention - #771 via @alfhenrik * Fixed: helper functions and cleanup to make using Authorization API easier to consume - #786 via @haacked From c344dfe7c286b8ff0cbf2974ab2dfd18c4b6a35a Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Thu, 23 Apr 2015 09:09:17 +0930 Subject: [PATCH 29/54] Revert "removed GetAllPublic overload due to paging issue and parameters clobbering" This reverts commit 26a8bf0e818ad8a620987e13d5ed7b1b8bacfa01. --- .../Clients/IObservableRepositoriesClient.cs | 12 ++++ .../Clients/ObservableRepositoriesClient.cs | 15 +++++ .../Clients/RepositoriesClientTests.cs | 24 ++++---- .../ObservableRepositoriesClientTests.cs | 17 ++++++ .../Clients/RepositoriesClientTests.cs | 31 ++++++++++ .../ObservableRepositoriesClientTests.cs | 59 +++++++++++++++++++ Octokit/Clients/IRepositoriesClient.cs | 14 +++++ Octokit/Clients/RepositoriesClient.cs | 18 ++++++ 8 files changed, 178 insertions(+), 12 deletions(-) diff --git a/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs b/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs index 9460a197..25337bab 100644 --- a/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs +++ b/Octokit.Reactive/Clients/IObservableRepositoriesClient.cs @@ -50,6 +50,18 @@ namespace Octokit.Reactive Justification = "Makes a network request")] IObservable GetAllPublic(); + /// + /// Retrieves every public since the last repository seen. + /// + /// + /// The default page size on GitHub.com is 30. + /// + /// Search parameters of the last repository seen + /// A of . + [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate", + Justification = "Makes a network request")] + IObservable GetAllPublic(PublicRepositoryRequest request); + /// /// Retrieves every that belongs to the current user. /// diff --git a/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs b/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs index 9c17dbf8..1a173d9c 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs @@ -102,6 +102,21 @@ namespace Octokit.Reactive return _connection.GetAndFlattenAllPages(ApiUrls.AllPublicRepositories()); } + /// + /// Retrieves every public since the last repository seen. + /// + /// + /// The default page size on GitHub.com is 30. + /// + /// Search parameters of the last repository seen + /// A of . + public IObservable GetAllPublic(PublicRepositoryRequest request) + { + Ensure.ArgumentNotNull(request, "request"); + + return _connection.GetAndFlattenAllPages(ApiUrls.AllPublicRepositories(), request.ToParametersDictionary()); + } + /// /// Retrieves every that belongs to the current user. /// diff --git a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs index b6c935e2..6c40f537 100644 --- a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs @@ -553,20 +553,20 @@ public class RepositoriesClientTests Assert.True(repositories.Count > 80); } - //[IntegrationTest] - //public async Task ReturnsAllPublicRepositoriesSinceLastSeen() - //{ - // var github = Helper.GetAuthenticatedClient(); + [IntegrationTest] + public async Task ReturnsAllPublicRepositoriesSinceLastSeen() + { + var github = Helper.GetAuthenticatedClient(); - // var request = new PublicRepositoryRequest(32732250); - // var repositories = await github.Repository.GetAllPublic(request); + var request = new PublicRepositoryRequest(32732250); + var repositories = await github.Repository.GetAllPublic(request); - // Assert.NotNull(repositories); - // Assert.True(repositories.Any()); - // Assert.Equal(32732252, repositories[0].Id); - // Assert.False(repositories[0].Private); - // Assert.Equal("zad19", repositories[0].Name); - //} + Assert.NotNull(repositories); + Assert.True(repositories.Any()); + Assert.Equal(32732252, repositories[0].Id); + Assert.False(repositories[0].Private); + Assert.Equal("zad19", repositories[0].Name); + } } public class TheGetAllForOrgMethod diff --git a/Octokit.Tests.Integration/Reactive/ObservableRepositoriesClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableRepositoriesClientTests.cs index 68341f58..397eb663 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableRepositoriesClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableRepositoriesClientTests.cs @@ -28,5 +28,22 @@ namespace Octokit.Tests.Integration Assert.False(repository2.Fork); } } + + public class TheGetAllPublicSinceMethod + { + [IntegrationTest(Skip = "This will take a very long time to return, so will skip it for now.")] + public async Task ReturnsAllPublicReposSinceLastSeen() + { + var github = Helper.GetAuthenticatedClient(); + + var client = new ObservableRepositoriesClient(github); + var request = new PublicRepositoryRequest(32732250); + var repositories = await client.GetAllPublic(request).ToArray(); + Assert.NotEmpty(repositories); + Assert.Equal(32732252, repositories[0].Id); + Assert.False(repositories[0].Private); + Assert.Equal("zad19", repositories[0].Name); + } + } } } diff --git a/Octokit.Tests/Clients/RepositoriesClientTests.cs b/Octokit.Tests/Clients/RepositoriesClientTests.cs index 0534472b..ed73c0f7 100644 --- a/Octokit.Tests/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoriesClientTests.cs @@ -273,6 +273,37 @@ namespace Octokit.Tests.Clients } } + + public class TheGetAllPublicSinceMethod + { + [Fact] + public void RequestsTheCorrectUrl() + { + var connection = Substitute.For(); + var client = new RepositoriesClient(connection); + + client.GetAllPublic(new PublicRepositoryRequest(364)); + + connection.Received() + .GetAll(Arg.Is(u => u.ToString() == "/repositories"), + Arg.Any>()); + } + + [Fact] + public void SendsTheCorrectParameter() + { + var connection = Substitute.For(); + var client = new RepositoriesClient(connection); + + client.GetAllPublic(new PublicRepositoryRequest(364)); + + connection.Received() + .GetAll(Arg.Is(u => u.ToString() == "/repositories"), + Arg.Is>(d => d.Count == 1 + && d["since"] == "364")); + } + } + public class TheGetAllForCurrentMethod { [Fact] diff --git a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs index 044a74d7..d1668b14 100644 --- a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs @@ -158,6 +158,65 @@ namespace Octokit.Tests.Reactive } } + public class TheGetAllPublicRepositoriesSinceMethod + { + [Fact] + public async Task ReturnsEveryPageOfRepositories() + { + var firstPageUrl = new Uri("/repositories", UriKind.Relative); + var secondPageUrl = new Uri("https://example.com/page/2"); + var firstPageLinks = new Dictionary { { "next", secondPageUrl } }; + IApiResponse> firstPageResponse = new ApiResponse>( + CreateResponseWithApiInfo(firstPageLinks), + new List + { + new Repository(364), + new Repository(365), + new Repository(366) + }); + + var thirdPageUrl = new Uri("https://example.com/page/3"); + var secondPageLinks = new Dictionary { { "next", thirdPageUrl } }; + IApiResponse> secondPageResponse = new ApiResponse> + ( + CreateResponseWithApiInfo(secondPageLinks), + new List + { + new Repository(367), + new Repository(368), + new Repository(369) + }); + + IApiResponse> lastPageResponse = new ApiResponse>( + new Response(), + new List + { + new Repository(370) + }); + + var gitHubClient = Substitute.For(); + gitHubClient.Connection.Get>(firstPageUrl, + Arg.Is>(d => d.Count == 1 + && d["since"] == "364"), null) + .Returns(Task.FromResult(firstPageResponse)); + gitHubClient.Connection.Get>(secondPageUrl, null, null) + .Returns(Task.FromResult(secondPageResponse)); + gitHubClient.Connection.Get>(thirdPageUrl, null, null) + .Returns(Task.FromResult(lastPageResponse)); + + var repositoriesClient = new ObservableRepositoriesClient(gitHubClient); + + var results = await repositoriesClient.GetAllPublic(new PublicRepositoryRequest(364)).ToArray(); + + Assert.Equal(7, results.Length); + gitHubClient.Connection.Received(1).Get>(firstPageUrl, + Arg.Is>(d=>d.Count == 1 + && d["since"] == "364"), null); + gitHubClient.Connection.Received(1).Get>(secondPageUrl, null, null); + gitHubClient.Connection.Received(1).Get>(thirdPageUrl, null, null); + } + } + public class TheGetAllBranchesMethod { [Fact] diff --git a/Octokit/Clients/IRepositoriesClient.cs b/Octokit/Clients/IRepositoriesClient.cs index 7172f0f4..e6f07092 100644 --- a/Octokit/Clients/IRepositoriesClient.cs +++ b/Octokit/Clients/IRepositoriesClient.cs @@ -109,6 +109,20 @@ namespace Octokit Justification = "Makes a network request")] Task> GetAllPublic(); + + /// + /// Gets all public repositories since the integer ID of the last Repository that you've seen. + /// + /// + /// See the API documentation for more information. + /// The default page size on GitHub.com is 30. + /// + /// Search parameters of the last repository seen + /// Thrown if the client is not authenticated. + /// Thrown when a general API error occurs. + /// A of . + Task> GetAllPublic(PublicRepositoryRequest request); + /// /// Gets all repositories owned by the current user. /// diff --git a/Octokit/Clients/RepositoriesClient.cs b/Octokit/Clients/RepositoriesClient.cs index 5de9ceec..479ac89f 100644 --- a/Octokit/Clients/RepositoriesClient.cs +++ b/Octokit/Clients/RepositoriesClient.cs @@ -188,6 +188,24 @@ namespace Octokit return ApiConnection.GetAll(ApiUrls.AllPublicRepositories()); } + /// + /// Gets all public repositories since the integer ID of the last Repository that you've seen. + /// + /// + /// See the API documentation for more information. + /// The default page size on GitHub.com is 30. + /// + /// Search parameters of the last repository seen + /// Thrown if the client is not authenticated. + /// Thrown when a general API error occurs. + /// A of . + public Task> GetAllPublic(PublicRepositoryRequest request) + { + Ensure.ArgumentNotNull(request, "request"); + + return ApiConnection.GetAll(ApiUrls.AllPublicRepositories(), request.ToParametersDictionary()); + } + /// /// Gets all repositories owned by the current user. /// From 2eb8e562eea56094122f7dd26bf28490d2c3ecd0 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Thu, 23 Apr 2015 09:14:06 +0930 Subject: [PATCH 30/54] avoid using parameters here, just craft the URL by hand --- .../Clients/ObservableRepositoriesClient.cs | 4 +++- Octokit/Clients/RepositoriesClient.cs | 4 +++- Octokit/Helpers/ApiUrls.cs | 14 +++++++++++--- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs b/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs index 1a173d9c..9f767b51 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoriesClient.cs @@ -114,7 +114,9 @@ namespace Octokit.Reactive { Ensure.ArgumentNotNull(request, "request"); - return _connection.GetAndFlattenAllPages(ApiUrls.AllPublicRepositories(), request.ToParametersDictionary()); + var url = ApiUrls.AllPublicRepositories(request.Since); + + return _connection.GetAndFlattenAllPages(url); } /// diff --git a/Octokit/Clients/RepositoriesClient.cs b/Octokit/Clients/RepositoriesClient.cs index 479ac89f..979ec01b 100644 --- a/Octokit/Clients/RepositoriesClient.cs +++ b/Octokit/Clients/RepositoriesClient.cs @@ -203,7 +203,9 @@ namespace Octokit { Ensure.ArgumentNotNull(request, "request"); - return ApiConnection.GetAll(ApiUrls.AllPublicRepositories(), request.ToParametersDictionary()); + var url = ApiUrls.AllPublicRepositories(request.Since); + + return ApiConnection.GetAll(url); } /// diff --git a/Octokit/Helpers/ApiUrls.cs b/Octokit/Helpers/ApiUrls.cs index 475ae0cc..f42096ea 100644 --- a/Octokit/Helpers/ApiUrls.cs +++ b/Octokit/Helpers/ApiUrls.cs @@ -19,15 +19,23 @@ namespace Octokit static readonly Uri _oauthAuthorize = new Uri("login/oauth/authorize", UriKind.Relative); static readonly Uri _oauthAccesToken = new Uri("login/oauth/access_token", UriKind.Relative); + /// + /// Returns the that returns all public repositories in + /// response to a GET request. + /// + public static Uri AllPublicRepositories() + { + return "/repositories".FormatUri(); + } /// /// Returns the that returns all public repositories in /// response to a GET request. /// - /// - public static Uri AllPublicRepositories() + /// The integer ID of the last Repository that you’ve seen. + public static Uri AllPublicRepositories(long since) { - return "/repositories".FormatUri(); + return "/repositories?since={0}".FormatUri(since); } /// From 446fb1b2c062876e148a5d8030090af385855a17 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Thu, 23 Apr 2015 09:17:08 +0930 Subject: [PATCH 31/54] needs a bit more pagination --- Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs index 6c40f537..6f3b5344 100644 --- a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs @@ -553,7 +553,7 @@ public class RepositoriesClientTests Assert.True(repositories.Count > 80); } - [IntegrationTest] + [IntegrationTest(Skip = "Takes too long to run.")] public async Task ReturnsAllPublicRepositoriesSinceLastSeen() { var github = Helper.GetAuthenticatedClient(); From 6ab19de0516b535061024a60746b79fa36235295 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Thu, 23 Apr 2015 10:18:10 +0930 Subject: [PATCH 32/54] what is tests --- Octokit.Tests/Clients/RepositoriesClientTests.cs | 7 ++----- .../Reactive/ObservableRepositoriesClientTests.cs | 10 +++------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/Octokit.Tests/Clients/RepositoriesClientTests.cs b/Octokit.Tests/Clients/RepositoriesClientTests.cs index ed73c0f7..1576fb18 100644 --- a/Octokit.Tests/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoriesClientTests.cs @@ -285,8 +285,7 @@ namespace Octokit.Tests.Clients client.GetAllPublic(new PublicRepositoryRequest(364)); connection.Received() - .GetAll(Arg.Is(u => u.ToString() == "/repositories"), - Arg.Any>()); + .GetAll(Arg.Is(u => u.ToString() == "/repositories?since=364")); } [Fact] @@ -298,9 +297,7 @@ namespace Octokit.Tests.Clients client.GetAllPublic(new PublicRepositoryRequest(364)); connection.Received() - .GetAll(Arg.Is(u => u.ToString() == "/repositories"), - Arg.Is>(d => d.Count == 1 - && d["since"] == "364")); + .GetAll(Arg.Is(u => u.ToString() == "/repositories?since=364")); } } diff --git a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs index d1668b14..f0ea85b1 100644 --- a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs @@ -163,7 +163,7 @@ namespace Octokit.Tests.Reactive [Fact] public async Task ReturnsEveryPageOfRepositories() { - var firstPageUrl = new Uri("/repositories", UriKind.Relative); + var firstPageUrl = new Uri("/repositories?since=364", UriKind.Relative); var secondPageUrl = new Uri("https://example.com/page/2"); var firstPageLinks = new Dictionary { { "next", secondPageUrl } }; IApiResponse> firstPageResponse = new ApiResponse>( @@ -195,9 +195,7 @@ namespace Octokit.Tests.Reactive }); var gitHubClient = Substitute.For(); - gitHubClient.Connection.Get>(firstPageUrl, - Arg.Is>(d => d.Count == 1 - && d["since"] == "364"), null) + gitHubClient.Connection.Get>(firstPageUrl, null, null) .Returns(Task.FromResult(firstPageResponse)); gitHubClient.Connection.Get>(secondPageUrl, null, null) .Returns(Task.FromResult(secondPageResponse)); @@ -209,9 +207,7 @@ namespace Octokit.Tests.Reactive var results = await repositoriesClient.GetAllPublic(new PublicRepositoryRequest(364)).ToArray(); Assert.Equal(7, results.Length); - gitHubClient.Connection.Received(1).Get>(firstPageUrl, - Arg.Is>(d=>d.Count == 1 - && d["since"] == "364"), null); + gitHubClient.Connection.Received(1).Get>(firstPageUrl, null, null); gitHubClient.Connection.Received(1).Get>(secondPageUrl, null, null); gitHubClient.Connection.Received(1).Get>(thirdPageUrl, null, null); } From 88e6c92bb21e28d57976cf47769a47ba35541efe Mon Sep 17 00:00:00 2001 From: Dillon Buchanan Date: Sun, 26 Apr 2015 10:20:36 -0400 Subject: [PATCH 33/54] Renamed PullRequestFile's *Uri properties to *Url --- Octokit/Models/Response/PullRequestFile.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Octokit/Models/Response/PullRequestFile.cs b/Octokit/Models/Response/PullRequestFile.cs index 703a0477..efa97899 100644 --- a/Octokit/Models/Response/PullRequestFile.cs +++ b/Octokit/Models/Response/PullRequestFile.cs @@ -14,7 +14,7 @@ namespace Octokit { public PullRequestFile() { } - public PullRequestFile(string sha, string fileName, string status, int additions, int deletions, int changes, Uri blobUri, Uri rawUri, Uri contentsUri, string patch) + public PullRequestFile(string sha, string fileName, string status, int additions, int deletions, int changes, Uri blobUrl, Uri rawUrl, Uri contentsUrl, string patch) { Sha = sha; FileName = fileName; @@ -22,9 +22,9 @@ namespace Octokit Additions = additions; Deletions = deletions; Changes = changes; - BlobUri = blobUri; - RawUri = rawUri; - ContentsUri = contentsUri; + BlobUrl = blobUrl; + RawUrl = rawUrl; + ContentsUrl = contentsUrl; Patch = patch; } @@ -35,9 +35,9 @@ namespace Octokit public int Additions { get; protected set; } public int Deletions { get; protected set; } public int Changes { get; protected set; } - public Uri BlobUri { get; protected set; } - public Uri RawUri { get; protected set; } - public Uri ContentsUri { get; protected set; } + public Uri BlobUrl { get; protected set; } + public Uri RawUrl { get; protected set; } + public Uri ContentsUrl { get; protected set; } public string Patch { get; protected set; } internal string DebuggerDisplay From c710c466764d0a097d94fe9a8a4ee866d96df658 Mon Sep 17 00:00:00 2001 From: csware Date: Mon, 27 Apr 2015 18:58:09 +0200 Subject: [PATCH 34/54] Allow to download zip-attachments Fixes issue #784. --- Octokit/Http/HttpClientAdapter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index ec7ffd4b..94cb4a90 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -76,8 +76,8 @@ namespace Octokit.Internal { contentType = GetContentMediaType(responseMessage.Content); - // We added support for downloading images. Let's constrain this appropriately. - if (contentType == null || !contentType.StartsWith("image/")) + // We added support for downloading images and zip-files. Let's constrain this appropriately. + if (contentType == null || (!contentType.StartsWith("image/") && !contentType.StartsWith("application/"))) { responseBody = await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false); } From fc8319cc338b1647165b2ed4ab07b367081b368b Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Fri, 24 Apr 2015 09:44:50 +0930 Subject: [PATCH 35/54] define a cancellation token for the request --- Octokit/Http/HttpClientAdapter.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index 94cb4a90..759dce0d 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -50,15 +50,25 @@ namespace Octokit.Internal httpOptions.Proxy = _webProxy; } + var cancellationTokenForRequest = cancellationToken; + + if (request.Timeout != TimeSpan.Zero) + { + var timeoutCancellation = new CancellationTokenSource(request.Timeout); + var unifiedCancellationToken = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCancellation.Token); + + cancellationTokenForRequest = unifiedCancellationToken.Token; + } + var http = new HttpClient(httpOptions) { - BaseAddress = request.BaseAddress, - Timeout = request.Timeout + BaseAddress = request.BaseAddress }; + using (var requestMessage = BuildRequestMessage(request)) { // Make the request - var responseMessage = await http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationToken) + var responseMessage = await http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationTokenForRequest) .ConfigureAwait(false); return await BuildResponse(responseMessage).ConfigureAwait(false); } From 67789e7a207985b28b4d0cbdae6a0a9a21a4cccd Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Fri, 24 Apr 2015 09:52:42 +0930 Subject: [PATCH 36/54] wrote a test, the world didn't end --- .../HttpClientAdapterTests.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Octokit.Tests.Integration/HttpClientAdapterTests.cs b/Octokit.Tests.Integration/HttpClientAdapterTests.cs index d721c752..bf4a0fb4 100644 --- a/Octokit.Tests.Integration/HttpClientAdapterTests.cs +++ b/Octokit.Tests.Integration/HttpClientAdapterTests.cs @@ -32,5 +32,25 @@ public class HttpClientAdapterTests Assert.Equal(78, imageBytes[2]); Assert.Equal(130, imageBytes.Last()); } + + [IntegrationTest] + public async Task CanCancelARequest() + { + var httpClient = new HttpClientAdapter(); + var request = new Request + { + BaseAddress = new Uri("https://github.global.ssl.fastly.net/", UriKind.Absolute), + Endpoint = new Uri("/images/icons/emoji/poop.png?v=5", UriKind.RelativeOrAbsolute), + AllowAutoRedirect = true, + Method = HttpMethod.Get, + Timeout = TimeSpan.FromMilliseconds(10) + }; + + var response = httpClient.Send(request, CancellationToken.None); + + await Task.Delay(TimeSpan.FromSeconds(2)); + + Assert.True(response.IsCanceled); + } } } From a9abd0942e4c2d0c3b9d149f76455589e7d725fd Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Fri, 24 Apr 2015 09:58:08 +0930 Subject: [PATCH 37/54] move address generation into message setup --- Octokit/Http/HttpClientAdapter.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index 759dce0d..4d3f5905 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -60,10 +60,7 @@ namespace Octokit.Internal cancellationTokenForRequest = unifiedCancellationToken.Token; } - var http = new HttpClient(httpOptions) - { - BaseAddress = request.BaseAddress - }; + var http = new HttpClient(httpOptions); using (var requestMessage = BuildRequestMessage(request)) { @@ -111,7 +108,8 @@ namespace Octokit.Internal HttpRequestMessage requestMessage = null; try { - requestMessage = new HttpRequestMessage(request.Method, request.Endpoint); + var fullUri = new Uri(request.BaseAddress, request.Endpoint); + requestMessage = new HttpRequestMessage(request.Method, fullUri); foreach (var header in request.Headers) { requestMessage.Headers.Add(header.Key, header.Value); From 288839543d5454187dcd250fd29332b7f5755fa7 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Fri, 8 May 2015 17:59:28 +0930 Subject: [PATCH 38/54] :lipstick: moving things --- Octokit/Http/HttpClientAdapter.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index 4d3f5905..1cc2be3a 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -50,6 +50,7 @@ namespace Octokit.Internal httpOptions.Proxy = _webProxy; } + var http = new HttpClient(httpOptions); var cancellationTokenForRequest = cancellationToken; if (request.Timeout != TimeSpan.Zero) @@ -60,8 +61,6 @@ namespace Octokit.Internal cancellationTokenForRequest = unifiedCancellationToken.Token; } - var http = new HttpClient(httpOptions); - using (var requestMessage = BuildRequestMessage(request)) { // Make the request From 09d1932b4acc010a13d67e5513b7d0c32e04ec89 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sat, 21 Mar 2015 21:54:01 +1000 Subject: [PATCH 39/54] Add method for getting repo archive link --- Octokit/Clients/IRepositoryContentsClient.cs | 37 ++++++++++++++++++++ Octokit/Clients/RepositoryContentsClient.cs | 36 +++++++++++++++++++ Octokit/Helpers/ApiUrls.cs | 5 +++ Octokit/Http/ApiConnection.cs | 14 ++++++++ Octokit/Http/IApiConnection.cs | 2 ++ 5 files changed, 94 insertions(+) diff --git a/Octokit/Clients/IRepositoryContentsClient.cs b/Octokit/Clients/IRepositoryContentsClient.cs index 84b7db96..c9d1978f 100644 --- a/Octokit/Clients/IRepositoryContentsClient.cs +++ b/Octokit/Clients/IRepositoryContentsClient.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using System.Threading.Tasks; +using Octokit.Internal; namespace Octokit { @@ -46,6 +47,32 @@ namespace Octokit /// Task GetReadmeHtml(string owner, string name); + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// + Task GetArchiveLink(string owner, string name); + + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A valid Git reference. + /// + Task GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference); + /// /// Creates a commit that creates a new file in a repository. /// @@ -75,4 +102,14 @@ namespace Octokit /// Information about the file to delete Task DeleteFile(string owner, string name, string path, DeleteFileRequest request); } + + public enum ArchiveFormat + { + [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Tarball")] + [Parameter(Value = "tarball")] + Tarball, + [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Zipball")] + [Parameter(Value = "zipball")] + Zipball + } } \ No newline at end of file diff --git a/Octokit/Clients/RepositoryContentsClient.cs b/Octokit/Clients/RepositoryContentsClient.cs index 5cf1caae..b8d95913 100644 --- a/Octokit/Clients/RepositoryContentsClient.cs +++ b/Octokit/Clients/RepositoryContentsClient.cs @@ -74,6 +74,42 @@ namespace Octokit return ApiConnection.GetHtml(ApiUrls.RepositoryReadme(owner, name), null); } + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// + public Task GetArchiveLink(string owner, string name) + { + return GetArchiveLink(owner, name, ArchiveFormat.Tarball, "master"); + } + + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A valid Git reference. + /// + public Task GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNullOrEmptyString(reference, "reference"); + + return ApiConnection.GetRedirect(ApiUrls.RepositoryArchiveLink(owner, name, archiveFormat, reference)); + } + /// /// Creates a commit that creates a new file in a repository. /// diff --git a/Octokit/Helpers/ApiUrls.cs b/Octokit/Helpers/ApiUrls.cs index fbf71833..357b66c3 100644 --- a/Octokit/Helpers/ApiUrls.cs +++ b/Octokit/Helpers/ApiUrls.cs @@ -1467,5 +1467,10 @@ namespace Octokit { return "repos/{0}/{1}/contents/{2}".FormatUri(owner, name, path); } + + public static Uri RepositoryArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference) + { + return "repos/{0}/{1}/{2}/{3}".FormatUri(owner, name, archiveFormat.ToParameter(), reference); + } } } diff --git a/Octokit/Http/ApiConnection.cs b/Octokit/Http/ApiConnection.cs index 145ef8ab..88f968db 100644 --- a/Octokit/Http/ApiConnection.cs +++ b/Octokit/Http/ApiConnection.cs @@ -386,6 +386,20 @@ namespace Octokit return Connection.Delete(uri, data); } + public async Task GetRedirect(Uri uri) + { + Ensure.ArgumentNotNull(uri, "uri"); + var response = await Connection.GetResponse(uri); + + if (response.HttpResponse.StatusCode == HttpStatusCode.Redirect) + { + return response.HttpResponse.Headers["Location"]; + } + + throw new ApiException("Redirect Operation expect status code or Redirect.", + response.HttpResponse.StatusCode); + } + /// /// Executes a GET to the API object at the specified URI. This operation is appropriate for /// API calls which queue long running calculations. diff --git a/Octokit/Http/IApiConnection.cs b/Octokit/Http/IApiConnection.cs index 9c6fee79..6c9583bd 100644 --- a/Octokit/Http/IApiConnection.cs +++ b/Octokit/Http/IApiConnection.cs @@ -232,6 +232,8 @@ namespace Octokit /// A for the request's execution. Task Delete(Uri uri, object data); + Task GetRedirect(Uri uri); + /// /// Executes a GET to the API object at the specified URI. This operation is appropriate for /// API calls which queue long running calculations. From 872f1f03caf8c4e84dc2b11699b6d2bb7aad2662 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sat, 21 Mar 2015 21:54:57 +1000 Subject: [PATCH 40/54] Add unit tests --- .../Clients/RepositoryContentsClientTests.cs | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/Octokit.Tests/Clients/RepositoryContentsClientTests.cs b/Octokit.Tests/Clients/RepositoryContentsClientTests.cs index 48df1bee..9a90455e 100644 --- a/Octokit.Tests/Clients/RepositoryContentsClientTests.cs +++ b/Octokit.Tests/Clients/RepositoryContentsClientTests.cs @@ -2,6 +2,7 @@ using System.Text; using System.Threading.Tasks; using NSubstitute; +using Octokit.Tests.Helpers; using Xunit; namespace Octokit.Tests.Clients @@ -52,6 +53,49 @@ namespace Octokit.Tests.Clients connection.Received().GetHtml(Arg.Is(u => u.ToString() == "repos/fake/repo/readme"), null); Assert.Equal("README", readme); } - } + } + + public class TheGetArchiveLinkMethod + { + [Fact] + public async Task ReturnsArchiveLinkWithDefaults() + { + var connection = Substitute.For(); + connection.GetRedirect(Args.Uri).Returns(Task.FromResult("https://codeload.github.com/fake/repo/legacy.tar.gz/master")); + var contentsClient = new RepositoryContentsClient(connection); + + var archiveLink = await contentsClient.GetArchiveLink("fake", "repo"); + + connection.Received().GetRedirect(Arg.Is(u => u.ToString() == "repos/fake/repo/tarball/master")); + Assert.Equal("https://codeload.github.com/fake/repo/legacy.tar.gz/master", archiveLink); + } + + [Fact] + public async Task ReturnsArchiveLinkWithSpecifiedValues() + { + var connection = Substitute.For(); + connection.GetRedirect(Args.Uri).Returns(Task.FromResult("https://codeload.github.com/fake/repo/legazy.zip/release")); + var contentsClient = new RepositoryContentsClient(connection); + + var archiveLink = await contentsClient.GetArchiveLink("fake", "repo", ArchiveFormat.Zipball, "release"); + + connection.Received().GetRedirect(Arg.Is(u => u.ToString() == "repos/fake/repo/zipball/release")); + Assert.Equal("https://codeload.github.com/fake/repo/legazy.zip/release", archiveLink); + } + + [Fact] + public async Task EnsuresArgumentsNotNull() + { + var connection = Substitute.For(); + var contentsClient = new RepositoryContentsClient(connection); + + AssertEx.Throws(async () => await contentsClient.GetArchiveLink(null, "name")); + AssertEx.Throws(async () => await contentsClient.GetArchiveLink("owner", null)); + AssertEx.Throws(async () => await contentsClient.GetArchiveLink("owner", "name", ArchiveFormat.Tarball, null)); + AssertEx.Throws(async () => await contentsClient.GetArchiveLink("", "name")); + AssertEx.Throws(async () => await contentsClient.GetArchiveLink("owner", "")); + AssertEx.Throws(async () => await contentsClient.GetArchiveLink("owner", "name", ArchiveFormat.Zipball, "")); + } + } } } \ No newline at end of file From 3a915fe6cfd6061aa1d3a12ee8f79b3f79e60166 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sat, 21 Mar 2015 23:18:32 +1000 Subject: [PATCH 41/54] :vertical_traffic_light: for following redirects --- Octokit/Clients/RepositoryContentsClient.cs | 1 - Octokit/Helpers/ApiExtensions.cs | 8 ++++++++ Octokit/Http/ApiConnection.cs | 12 +++++++++-- Octokit/Http/Connection.cs | 22 ++++++++++++++++++++- Octokit/Http/IApiConnection.cs | 8 ++++++++ Octokit/Http/IConnection.cs | 13 ++++++++++++ 6 files changed, 60 insertions(+), 4 deletions(-) diff --git a/Octokit/Clients/RepositoryContentsClient.cs b/Octokit/Clients/RepositoryContentsClient.cs index b8d95913..a69b31f2 100644 --- a/Octokit/Clients/RepositoryContentsClient.cs +++ b/Octokit/Clients/RepositoryContentsClient.cs @@ -105,7 +105,6 @@ namespace Octokit { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); Ensure.ArgumentNotNullOrEmptyString(name, "name"); - Ensure.ArgumentNotNullOrEmptyString(reference, "reference"); return ApiConnection.GetRedirect(ApiUrls.RepositoryArchiveLink(owner, name, archiveFormat, reference)); } diff --git a/Octokit/Helpers/ApiExtensions.cs b/Octokit/Helpers/ApiExtensions.cs index bf164a6b..032c7e3c 100644 --- a/Octokit/Helpers/ApiExtensions.cs +++ b/Octokit/Helpers/ApiExtensions.cs @@ -90,6 +90,14 @@ namespace Octokit return connection.Get(uri, null, null); } + public static Task> GetRedirect(this IConnection connection, Uri uri) + { + Ensure.ArgumentNotNull(connection, "connection"); + Ensure.ArgumentNotNull(uri, "uri"); + + return connection.Get(uri, null, null, false); + } + /// /// Gets the API resource at the specified URI. /// diff --git a/Octokit/Http/ApiConnection.cs b/Octokit/Http/ApiConnection.cs index 88f968db..f53df010 100644 --- a/Octokit/Http/ApiConnection.cs +++ b/Octokit/Http/ApiConnection.cs @@ -386,17 +386,25 @@ namespace Octokit return Connection.Delete(uri, data); } + /// + /// Executes a GET to the API object at the specified URI. This operation is appropriate for + /// API calls which wants to return the redirect URL. + /// It expects the API to respond with a 302 Found. + /// + /// URI of the API resource to get + /// The URL returned by the API in the Location header + /// Thrown when an API error occurs, or the API does not respond with a 302 Found public async Task GetRedirect(Uri uri) { Ensure.ArgumentNotNull(uri, "uri"); - var response = await Connection.GetResponse(uri); + var response = await Connection.GetRedirect(uri); if (response.HttpResponse.StatusCode == HttpStatusCode.Redirect) { return response.HttpResponse.Headers["Location"]; } - throw new ApiException("Redirect Operation expect status code or Redirect.", + throw new ApiException("Redirect Operation expect status code of Redirect.", response.HttpResponse.StatusCode); } diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 7d2f3e93..dfbc18a5 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -142,6 +142,24 @@ namespace Octokit return SendData(uri.ApplyParameters(parameters), HttpMethod.Get, null, accepts, null, CancellationToken.None); } + /// + /// Performs an asynchronous HTTP GET request. + /// Attempts to map the response to an object of type + /// + /// The type to map the response to + /// URI endpoint to send request to + /// Querystring parameters for the request + /// Specifies accepted response media types. + /// To follow redirect links automatically or not + /// representing the received HTTP response + + public Task> Get(Uri uri, IDictionary parameters, string accepts, bool allowAutoRedirect) + { + Ensure.ArgumentNotNull(uri, "uri"); + + return SendData(uri.ApplyParameters(parameters), HttpMethod.Get, null, accepts, null, CancellationToken.None, allowAutoRedirect: allowAutoRedirect); + } + public Task> Get(Uri uri, IDictionary parameters, string accepts, CancellationToken cancellationToken) { Ensure.ArgumentNotNull(uri, "uri"); @@ -289,7 +307,8 @@ namespace Octokit string contentType, CancellationToken cancellationToken, string twoFactorAuthenticationCode = null, - Uri baseAddress = null) + Uri baseAddress = null, + bool allowAutoRedirect = true) { Ensure.ArgumentNotNull(uri, "uri"); @@ -298,6 +317,7 @@ namespace Octokit Method = method, BaseAddress = baseAddress ?? BaseAddress, Endpoint = uri, + AllowAutoRedirect = allowAutoRedirect, }; return SendDataInternal(body, accepts, contentType, cancellationToken, twoFactorAuthenticationCode, request); diff --git a/Octokit/Http/IApiConnection.cs b/Octokit/Http/IApiConnection.cs index 6c9583bd..303604e7 100644 --- a/Octokit/Http/IApiConnection.cs +++ b/Octokit/Http/IApiConnection.cs @@ -232,6 +232,14 @@ namespace Octokit /// A for the request's execution. Task Delete(Uri uri, object data); + /// + /// Executes a GET to the API object at the specified URI. This operation is appropriate for + /// API calls which wants to return the redirect URL. + /// It expects the API to respond with a 302 Found. + /// + /// URI of the API resource to get + /// The URL returned by the API in the Location header + /// Thrown when an API error occurs, or the API does not respond with a 302 Found Task GetRedirect(Uri uri); /// diff --git a/Octokit/Http/IConnection.cs b/Octokit/Http/IConnection.cs index afc5b938..9c74fa9c 100644 --- a/Octokit/Http/IConnection.cs +++ b/Octokit/Http/IConnection.cs @@ -32,6 +32,19 @@ namespace Octokit [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get")] Task> Get(Uri uri, IDictionary parameters, string accepts); + /// + /// Performs an asynchronous HTTP GET request. + /// Attempts to map the response to an object of type + /// + /// The type to map the response to + /// URI endpoint to send request to + /// Querystring parameters for the request + /// Specifies accepted response media types. + /// To follow redirect links automatically or not + /// representing the received HTTP response + [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get")] + Task> Get(Uri uri, IDictionary parameters, string accepts, bool allowAutoRedirect); + /// /// Performs an asynchronous HTTP GET request. /// Attempts to map the response to an object of type From bac6463ed61fbf87aa737a2683b33dac0a63d6c8 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sat, 21 Mar 2015 23:18:40 +1000 Subject: [PATCH 42/54] Add integration tests --- .../Clients/RepositoryContentsClientTests.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs index 05bd0607..d2f949ad 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs @@ -128,5 +128,53 @@ namespace Octokit.Tests.Integration.Clients Helper.DeleteRepo(repository); } } + + [IntegrationTest] + public async Task GetsArchiveLinkAsTarball() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) + { + Credentials = Helper.Credentials + }; + + var archiveLink = await github + .Repository + .Content + .GetArchiveLink("octokit", "octokit.net"); + + Assert.Equal("https://codeload.github.com/octokit/octokit.net/legacy.tar.gz/master", archiveLink); + } + + [IntegrationTest] + public async Task GetsArchiveLinkAsZipball() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) + { + Credentials = Helper.Credentials + }; + + var archiveLink = await github + .Repository + .Content + .GetArchiveLink("octokit", "octokit.net", ArchiveFormat.Zipball, ""); + + Assert.Equal("https://codeload.github.com/octokit/octokit.net/legacy.zip/master", archiveLink); + } + + [IntegrationTest] + public async Task GetsArchiveLinkForReleaseBranchAsTarball() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) + { + Credentials = Helper.Credentials + }; + + var archiveLink = await github + .Repository + .Content + .GetArchiveLink("alfhenrik", "ScriptCs.OctoKit", ArchiveFormat.Tarball, "dev"); + + Assert.Equal("https://codeload.github.com/alfhenrik/ScriptCs.OctoKit/legacy.tar.gz/dev", archiveLink); + } } } \ No newline at end of file From 514ed613f3b68e90e86034870be477fffa52ea6b Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sat, 21 Mar 2015 23:32:58 +1000 Subject: [PATCH 43/54] Add another overload to GetArchiveLink --- Octokit/Clients/RepositoryContentsClient.cs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Octokit/Clients/RepositoryContentsClient.cs b/Octokit/Clients/RepositoryContentsClient.cs index a69b31f2..7b867f01 100644 --- a/Octokit/Clients/RepositoryContentsClient.cs +++ b/Octokit/Clients/RepositoryContentsClient.cs @@ -86,7 +86,23 @@ namespace Octokit /// public Task GetArchiveLink(string owner, string name) { - return GetArchiveLink(owner, name, ArchiveFormat.Tarball, "master"); + return GetArchiveLink(owner, name, ArchiveFormat.Tarball, string.Empty); + } + + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// + public Task GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat) + { + return GetArchiveLink(owner, name, archiveFormat, string.Empty); } /// From ce8afcad6f5c1c5c20cdbb9872fb5a9f9e8d9458 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sat, 21 Mar 2015 23:33:26 +1000 Subject: [PATCH 44/54] Add GetArchiveLink Observable client --- .../IObservableRepositoryContentsClient.cs | 39 ++++++++++++++ .../ObservableRepositoryContentsClient.cs | 52 +++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs b/Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs index bfe4cb3e..4b56ed1b 100644 --- a/Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs +++ b/Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs @@ -24,6 +24,45 @@ namespace Octokit.Reactive /// IObservable GetReadmeHtml(string owner, string name); + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// + IObservable GetArchiveLink(string owner, string name); + + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// + IObservable GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat); + + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A valid Git reference. + /// + IObservable GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference); + /// /// Returns the contents of a file or directory in a repository. /// diff --git a/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs b/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs index 0a45ca68..1a644aea 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs @@ -1,5 +1,6 @@ using System; using System.Reactive.Threading.Tasks; +using Microsoft.SqlServer.Server; using Octokit.Reactive.Internal; namespace Octokit.Reactive @@ -49,6 +50,57 @@ namespace Octokit.Reactive } + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// + public IObservable GetArchiveLink(string owner, string name) + { + return GetArchiveLink(owner, name, ArchiveFormat.Tarball, string.Empty); + } + + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// + public IObservable GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat) + { + return GetArchiveLink(owner, name, archiveFormat, String.Empty); + } + + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A valid Git reference. + /// + public IObservable GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + + return _client.Repository.Content.GetArchiveLink(owner, name, archiveFormat, reference).ToObservable(); + } + /// /// Returns the contents of a file or directory in a repository. /// From a5ce58b0c389e4d755e3d6c729e430009d8cc0d5 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sat, 21 Mar 2015 23:45:58 +1000 Subject: [PATCH 45/54] Fix 'em tests --- .../Clients/RepositoryContentsClientTests.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/Octokit.Tests/Clients/RepositoryContentsClientTests.cs b/Octokit.Tests/Clients/RepositoryContentsClientTests.cs index 9a90455e..6626f7ef 100644 --- a/Octokit.Tests/Clients/RepositoryContentsClientTests.cs +++ b/Octokit.Tests/Clients/RepositoryContentsClientTests.cs @@ -66,7 +66,20 @@ namespace Octokit.Tests.Clients var archiveLink = await contentsClient.GetArchiveLink("fake", "repo"); - connection.Received().GetRedirect(Arg.Is(u => u.ToString() == "repos/fake/repo/tarball/master")); + connection.Received().GetRedirect(Arg.Is(u => u.ToString() == "repos/fake/repo/tarball/")); + Assert.Equal("https://codeload.github.com/fake/repo/legacy.tar.gz/master", archiveLink); + } + + [Fact] + public async Task ReturnsArchiveLinkAsZipball() + { + var connection = Substitute.For(); + connection.GetRedirect(Args.Uri).Returns(Task.FromResult("https://codeload.github.com/fake/repo/legacy.tar.gz/master")); + var contentsClient = new RepositoryContentsClient(connection); + + var archiveLink = await contentsClient.GetArchiveLink("fake", "repo", ArchiveFormat.Zipball); + + connection.Received().GetRedirect(Arg.Is(u => u.ToString() == "repos/fake/repo/zipball/")); Assert.Equal("https://codeload.github.com/fake/repo/legacy.tar.gz/master", archiveLink); } @@ -91,10 +104,8 @@ namespace Octokit.Tests.Clients AssertEx.Throws(async () => await contentsClient.GetArchiveLink(null, "name")); AssertEx.Throws(async () => await contentsClient.GetArchiveLink("owner", null)); - AssertEx.Throws(async () => await contentsClient.GetArchiveLink("owner", "name", ArchiveFormat.Tarball, null)); AssertEx.Throws(async () => await contentsClient.GetArchiveLink("", "name")); AssertEx.Throws(async () => await contentsClient.GetArchiveLink("owner", "")); - AssertEx.Throws(async () => await contentsClient.GetArchiveLink("owner", "name", ArchiveFormat.Zipball, "")); } } } From 9ee9d4aad6645f6f8bbdcac15f8744a9a597d38b Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Sun, 22 Mar 2015 00:27:02 +1000 Subject: [PATCH 46/54] Ooh, so I needed that one :see_no_evil: --- Octokit/Clients/IRepositoryContentsClient.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Octokit/Clients/IRepositoryContentsClient.cs b/Octokit/Clients/IRepositoryContentsClient.cs index c9d1978f..45a217b0 100644 --- a/Octokit/Clients/IRepositoryContentsClient.cs +++ b/Octokit/Clients/IRepositoryContentsClient.cs @@ -59,6 +59,19 @@ namespace Octokit /// Task GetArchiveLink(string owner, string name); + /// + /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. + /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the + /// Location header to make a second GET request. + /// Note: For private repositories, these links are temporary and expire quickly. + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// + Task GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat); + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the From 0ac98e4135fbf8fecb6ef45626e180cab0657da1 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Thu, 23 Apr 2015 06:22:50 +1000 Subject: [PATCH 47/54] :do_not_litter: --- Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs b/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs index 1a644aea..ca2889e4 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs @@ -1,6 +1,5 @@ using System; using System.Reactive.Threading.Tasks; -using Microsoft.SqlServer.Server; using Octokit.Reactive.Internal; namespace Octokit.Reactive From fac6a01183d23c5ef65eb3dbd88b15a2eb948aeb Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Fri, 8 May 2015 18:11:00 +0930 Subject: [PATCH 48/54] update the tests based on the more rigorous requirements --- Octokit.Tests/Http/HttpClientAdapterTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Octokit.Tests/Http/HttpClientAdapterTests.cs b/Octokit.Tests/Http/HttpClientAdapterTests.cs index e1c76c48..029b7052 100644 --- a/Octokit.Tests/Http/HttpClientAdapterTests.cs +++ b/Octokit.Tests/Http/HttpClientAdapterTests.cs @@ -16,11 +16,15 @@ namespace Octokit.Tests.Http { public class TheBuildRequestMessageMethod { + readonly Uri _endpoint = new Uri("/ha-ha-business", UriKind.Relative); + [Fact] public void AddsHeadersToRequestMessage() { var request = new Request { + BaseAddress = GitHubClient.GitHubApiUrl, + Endpoint = _endpoint, Method = HttpMethod.Post, Headers = { @@ -47,6 +51,8 @@ namespace Octokit.Tests.Http { var request = new Request { + BaseAddress = GitHubClient.GitHubApiUrl, + Endpoint = _endpoint, Method = HttpMethod.Post, Body = "{}", ContentType = "text/plain" @@ -64,6 +70,8 @@ namespace Octokit.Tests.Http { var request = new Request { + BaseAddress = GitHubClient.GitHubApiUrl, + Endpoint = _endpoint, Method = HttpMethod.Post, Body = new MemoryStream(), ContentType = "text/plain" @@ -82,6 +90,8 @@ namespace Octokit.Tests.Http { var request = new Request { + BaseAddress = GitHubClient.GitHubApiUrl, + Endpoint = _endpoint, Method = HttpMethod.Post, Body = new FormUrlEncodedContent(new Dictionary {{"foo", "bar"}}) }; From b51598d8b1e9099c0e2217d1ac0feadf23dbb0ec Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Fri, 8 May 2015 18:25:58 +0930 Subject: [PATCH 49/54] run the specific checks first for known binary content --- Octokit/Http/HttpClientAdapter.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index 1cc2be3a..0e9888fc 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -83,13 +83,13 @@ namespace Octokit.Internal contentType = GetContentMediaType(responseMessage.Content); // We added support for downloading images and zip-files. Let's constrain this appropriately. - if (contentType == null || (!contentType.StartsWith("image/") && !contentType.StartsWith("application/"))) + if (contentType != null && (contentType.StartsWith("image/") || contentType.Equals("application/zip", StringComparison.OrdinalIgnoreCase))) { - responseBody = await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false); + responseBody = await responseMessage.Content.ReadAsByteArrayAsync().ConfigureAwait(false); } else { - responseBody = await responseMessage.Content.ReadAsByteArrayAsync().ConfigureAwait(false); + responseBody = await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false); } } } From 177290473e73a9a248cb2f25c7efa468d9b9f026 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Fri, 8 May 2015 18:26:13 +0930 Subject: [PATCH 50/54] by default you want the GitHub API --- Octokit/GitHubClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit/GitHubClient.cs b/Octokit/GitHubClient.cs index a1e5e67f..d88a6257 100644 --- a/Octokit/GitHubClient.cs +++ b/Octokit/GitHubClient.cs @@ -23,7 +23,7 @@ namespace Octokit /// the user agent for analytics purposes. /// public GitHubClient(ProductHeaderValue productInformation) - : this(new Connection(productInformation)) + : this(new Connection(productInformation, GitHubApiUrl)) { } From 021e482d748ecaa412bcf32b09fda6f4eb253d7f Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Fri, 8 May 2015 19:13:36 +1000 Subject: [PATCH 51/54] Use helper to get authd client --- .../Clients/RepositoryContentsClientTests.cs | 41 ++++--------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs index d2f949ad..707c4c47 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs @@ -12,10 +12,7 @@ namespace Octokit.Tests.Integration.Clients [IntegrationTest] public async Task ReturnsReadmeForSeeGit() { - var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) - { - Credentials = Helper.Credentials - }; + var github = Helper.GetAuthenticatedClient(); var readme = await github.Repository.Content.GetReadme("octokit", "octokit.net"); Assert.Equal("README.md", readme.Name); @@ -28,10 +25,7 @@ namespace Octokit.Tests.Integration.Clients [IntegrationTest] public async Task ReturnsReadmeHtmlForSeeGit() { - var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) - { - Credentials = Helper.Credentials - }; + var github = Helper.GetAuthenticatedClient(); var readmeHtml = await github.Repository.Content.GetReadmeHtml("octokit", "octokit.net"); Assert.True(readmeHtml.StartsWith("
Date: Sat, 9 May 2015 10:59:04 +0930 Subject: [PATCH 52/54] wrote some release notes --- ReleaseNotes.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ReleaseNotes.md b/ReleaseNotes.md index e9d39de7..9718262f 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -1,3 +1,10 @@ +### New in 0.11.0 (released 2015/05/10) +* New: Added overload to `IRepositoryClient.GetAllPublic` specifying a `since` parameter - #774 via @alfhenrik +* New: Added `IGistsClient.GetAllCommits` and `IGistsClient.GetAllForks` implementations - #542 via @haagenson, #794 via @shiftkey +* New: Added `IRepositoryContentsClient.GetArchiveLink` for getting archived code - #765 via @alfhenrik +* Fixed: `PullRequestFile` properties were not serialized correctly - #789 via @thedillonb +* Fixed: Allow to download zip-attachments - #792 via @csware + ### New in 0.10.0 (released 2015/04/22) * Fixed: renamed methods to follow `GetAll` convention - #771 via @alfhenrik * Fixed: helper functions and cleanup to make using Authorization API easier to consume - #786 via @haacked From aefe15c6dfc34c15581439b147bdf53c9cc23c5c Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Sat, 9 May 2015 11:00:07 +0930 Subject: [PATCH 53/54] version bump --- SolutionInfo.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SolutionInfo.cs b/SolutionInfo.cs index 78217389..b53121bd 100644 --- a/SolutionInfo.cs +++ b/SolutionInfo.cs @@ -3,11 +3,11 @@ using System.Reflection; using System.Runtime.InteropServices; [assembly: AssemblyProductAttribute("Octokit")] -[assembly: AssemblyVersionAttribute("0.10.0")] -[assembly: AssemblyFileVersionAttribute("0.10.0")] +[assembly: AssemblyVersionAttribute("0.11.0")] +[assembly: AssemblyFileVersionAttribute("0.11.0")] [assembly: ComVisibleAttribute(false)] namespace System { internal static class AssemblyVersionInformation { - internal const string Version = "0.10.0"; + internal const string Version = "0.11.0"; } } From 832e195a7e00eda6d7b5752fc787b028af64996b Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Sat, 9 May 2015 11:12:12 +0930 Subject: [PATCH 54/54] some additional tests can be run on these platforms --- Octokit.Tests/OctoKit.Tests-NetCore45.csproj | 2 ++ Octokit.Tests/Octokit.Tests-Portable.csproj | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Octokit.Tests/OctoKit.Tests-NetCore45.csproj b/Octokit.Tests/OctoKit.Tests-NetCore45.csproj index fafdc470..e83d502f 100644 --- a/Octokit.Tests/OctoKit.Tests-NetCore45.csproj +++ b/Octokit.Tests/OctoKit.Tests-NetCore45.csproj @@ -75,6 +75,7 @@ + @@ -147,6 +148,7 @@ + diff --git a/Octokit.Tests/Octokit.Tests-Portable.csproj b/Octokit.Tests/Octokit.Tests-Portable.csproj index 77072b10..04e328c8 100644 --- a/Octokit.Tests/Octokit.Tests-Portable.csproj +++ b/Octokit.Tests/Octokit.Tests-Portable.csproj @@ -75,6 +75,7 @@ + @@ -147,6 +148,7 @@ +