diff --git a/Octokit.Reactive/Clients/IObservableReleasesClient.cs b/Octokit.Reactive/Clients/IObservableReleasesClient.cs index a8bfa4cc..242e13d2 100644 --- a/Octokit.Reactive/Clients/IObservableReleasesClient.cs +++ b/Octokit.Reactive/Clients/IObservableReleasesClient.cs @@ -18,6 +18,19 @@ namespace Octokit.Reactive /// The list of s for the specified repository. IObservable GetAll(string owner, string name); + /// + /// Gets all s for the specified repository. + /// + /// + /// See the API documentation for more information. + /// + /// The repository's owner + /// The repository's name + /// Options for changing the API response + /// Thrown when a general API error occurs. + /// The list of s for the specified repository. + IObservable GetAll(string owner, string name, ApiOptions options); + /// /// Gets a single for the specified repository. /// diff --git a/Octokit.Reactive/Clients/ObservableReleasesClient.cs b/Octokit.Reactive/Clients/ObservableReleasesClient.cs index dfc9478e..100bd131 100644 --- a/Octokit.Reactive/Clients/ObservableReleasesClient.cs +++ b/Octokit.Reactive/Clients/ObservableReleasesClient.cs @@ -36,6 +36,26 @@ namespace Octokit.Reactive return _connection.GetAndFlattenAllPages(ApiUrls.Releases(owner, name)); } + /// + /// Gets all s for the specified repository. + /// + /// + /// See the API documentation for more information. + /// + /// The repository's owner + /// The repository's name + /// Options for changing the API response + /// Thrown when a general API error occurs. + /// The list of s for the specified repository. + public IObservable GetAll(string owner, string name, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNull(options, "options"); + + return _connection.GetAndFlattenAllPages(ApiUrls.Releases(owner, name), options); + } + /// /// Gets a single for the specified repository. /// diff --git a/Octokit.Reactive/Helpers/ConnectionExtensions.cs b/Octokit.Reactive/Helpers/ConnectionExtensions.cs index 0094134c..becbf01c 100644 --- a/Octokit.Reactive/Helpers/ConnectionExtensions.cs +++ b/Octokit.Reactive/Helpers/ConnectionExtensions.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using System.Globalization; +using System.Linq; using System.Reactive.Linq; using System.Reactive.Threading.Tasks; @@ -12,6 +14,15 @@ namespace Octokit.Reactive.Internal return GetPages(url, null, (pageUrl, pageParams) => connection.Get>(pageUrl, null, null).ToObservable()); } + public static IObservable GetAndFlattenAllPages(this IConnection connection, Uri url, ApiOptions options) + { + return GetPagesWithOptions(url, options, (pageUrl, o) => + { + var parameters = Pagination.Setup(new Dictionary(), options); + return connection.Get>(pageUrl, parameters, null).ToObservable(); + }); + } + public static IObservable GetAndFlattenAllPages(this IConnection connection, Uri url, IDictionary parameters) { return GetPages(url, parameters, (pageUrl, pageParams) => connection.Get>(pageUrl, pageParams, null).ToObservable()); @@ -35,5 +46,24 @@ namespace Octokit.Reactive.Internal .Where(resp => resp != null) .SelectMany(resp => resp.Body); } + + static IObservable GetPagesWithOptions(Uri uri, ApiOptions options, + Func>>> getPageFunc) + { + return getPageFunc(uri, options).Expand(resp => + { + var nextPageUri = resp.HttpResponse.ApiInfo.GetNextPageUrl(); + + var shouldContinue = Pagination.ShouldContinue( + nextPageUri, + options); + + return shouldContinue + ? Observable.Defer(() => getPageFunc(nextPageUri, null)) + : Observable.Empty>>(); + }) + .Where(resp => resp != null) + .SelectMany(resp => resp.Body); + } } } diff --git a/Octokit.Reactive/Octokit.Reactive-Mono.csproj b/Octokit.Reactive/Octokit.Reactive-Mono.csproj index 41db9810..6cf3a026 100644 --- a/Octokit.Reactive/Octokit.Reactive-Mono.csproj +++ b/Octokit.Reactive/Octokit.Reactive-Mono.csproj @@ -52,6 +52,9 @@ Helpers\Ensure.cs + + Helpers\Pagination.cs + Properties\SolutionInfo.cs @@ -181,4 +184,4 @@ Octokit-Mono - \ No newline at end of file + diff --git a/Octokit.Reactive/Octokit.Reactive-MonoAndroid.csproj b/Octokit.Reactive/Octokit.Reactive-MonoAndroid.csproj index 2a01d4da..146e66b0 100644 --- a/Octokit.Reactive/Octokit.Reactive-MonoAndroid.csproj +++ b/Octokit.Reactive/Octokit.Reactive-MonoAndroid.csproj @@ -60,6 +60,9 @@ Helpers\Ensure.cs + + Helpers\Pagination.cs + Properties\SolutionInfo.cs @@ -189,4 +192,4 @@ Octokit-MonoAndroid - \ No newline at end of file + diff --git a/Octokit.Reactive/Octokit.Reactive-Monotouch.csproj b/Octokit.Reactive/Octokit.Reactive-Monotouch.csproj index c3855b70..c3d64929 100644 --- a/Octokit.Reactive/Octokit.Reactive-Monotouch.csproj +++ b/Octokit.Reactive/Octokit.Reactive-Monotouch.csproj @@ -56,6 +56,9 @@ Helpers\Ensure.cs + + Helpers\Pagination.cs + Properties\SolutionInfo.cs @@ -185,4 +188,4 @@ Octokit-Monotouch - \ No newline at end of file + diff --git a/Octokit.Reactive/Octokit.Reactive.csproj b/Octokit.Reactive/Octokit.Reactive.csproj index 4c69b57f..8d1d8e99 100644 --- a/Octokit.Reactive/Octokit.Reactive.csproj +++ b/Octokit.Reactive/Octokit.Reactive.csproj @@ -72,6 +72,9 @@ Helpers\Ensure.cs + + Helpers\Pagination.cs + Properties\SolutionInfo.cs diff --git a/Octokit.Tests.Conventions/Exception/ApiOptionsMissingException.cs b/Octokit.Tests.Conventions/Exception/ApiOptionsMissingException.cs new file mode 100644 index 00000000..61d1111f --- /dev/null +++ b/Octokit.Tests.Conventions/Exception/ApiOptionsMissingException.cs @@ -0,0 +1,32 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; + +namespace Octokit.Tests.Conventions +{ + public class ApiOptionsMissingException : Exception + { + public ApiOptionsMissingException(Type type, IEnumerable methods) + : base(CreateMessage(type, methods)) { } + + static string CreateMessage(Type type, IEnumerable methods) + { + var methodsFormatted = String.Join("\r\n", methods.Select(FormatMethod)); + return "Methods found on type {0} require an overload which accepts an parameter of type ApiOptions:\r\n{1}" + .FormatWithNewLine( + type.Name, + methodsFormatted); + } + + static string FormatMethod(MethodInfo m) + { + var formattedParameters = m.GetParameters() + .Select(p => string.Format("{0} {1}", p.ParameterType.Name, p.Name)); + + var parameterList = string.Join(", ", formattedParameters); + + return string.Format(" - {0}({1})", m.Name, parameterList); + } + } +} diff --git a/Octokit.Tests.Conventions/ModelTests.cs b/Octokit.Tests.Conventions/ModelTests.cs index f126e8cd..0cfcb3c8 100644 --- a/Octokit.Tests.Conventions/ModelTests.cs +++ b/Octokit.Tests.Conventions/ModelTests.cs @@ -95,27 +95,6 @@ namespace Octokit.Tests.Conventions } } - //TODO: This should (probably) be moved to the PaginationTests class that is being introduced in PR #760 - [Theory] - [MemberData("GetClientInterfaces")] - public void CheckPaginationGetAllMethodNames(Type clientInterface) - { - var methodsOrdered = clientInterface.GetMethodsOrdered(); - - var methodsThatCanPaginate = methodsOrdered - .Where(x => x.ReturnType.GetTypeInfo().TypeCategory == TypeCategory.ReadOnlyList) - .Where(x => x.Name.StartsWith("Get")); - - var invalidMethods = methodsThatCanPaginate - .Where(x => !x.Name.StartsWith("GetAll")) - .ToList(); - - if (invalidMethods.Any()) - { - throw new PaginationGetAllMethodNameMismatchException(clientInterface, invalidMethods); - } - } - public static IEnumerable GetClientInterfaces() { return typeof(IEventsClient) diff --git a/Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj b/Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj index fe11b292..c6012156 100644 --- a/Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj +++ b/Octokit.Tests.Conventions/Octokit.Tests.Conventions.csproj @@ -68,6 +68,7 @@ + @@ -76,6 +77,7 @@ + diff --git a/Octokit.Tests.Conventions/PaginationTests.cs b/Octokit.Tests.Conventions/PaginationTests.cs new file mode 100644 index 00000000..3c2565e7 --- /dev/null +++ b/Octokit.Tests.Conventions/PaginationTests.cs @@ -0,0 +1,98 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using Xunit; + +namespace Octokit.Tests.Conventions +{ + public class PaginationTests + { + [Theory(Skip = "Enable this to run it and find all the places where things break")] + [MemberData("GetClientInterfaces")] + public void CheckObservableClients(Type clientInterface) + { + var methodsOrdered = clientInterface.GetMethodsOrdered(); + + var methodsWhichCanPaginate = methodsOrdered + .Where(x => x.Name.StartsWith("GetAll")); + + var invalidMethods = methodsWhichCanPaginate + .Where(method => MethodHasAppropriateOverload(method, methodsOrdered) == null) + .ToList(); + + if (invalidMethods.Any()) + { + throw new ApiOptionsMissingException(clientInterface, invalidMethods); + } + } + + [Theory] + [MemberData("GetClientInterfaces")] + public void CheckPaginationGetAllMethodNames(Type clientInterface) + { + var methodsOrdered = clientInterface.GetMethodsOrdered(); + + var methodsThatCanPaginate = methodsOrdered + .Where(x => x.ReturnType.GetTypeInfo().TypeCategory == TypeCategory.ReadOnlyList) + .Where(x => x.Name.StartsWith("Get")); + + var invalidMethods = methodsThatCanPaginate + .Where(x => !x.Name.StartsWith("GetAll")) + .ToList(); + + if (invalidMethods.Any()) + { + throw new PaginationGetAllMethodNameMismatchException(clientInterface, invalidMethods); + } + } + + static MethodInfo MethodHasAppropriateOverload(MethodInfo method, MethodInfo[] methodsOrdered) + { + var parameters = method.GetParametersOrdered(); + var name = method.Name; + return methodsOrdered + .Where(x => x.Name == name) + .FirstOrDefault(x => MethodHasOverloadForApiOptions(x, parameters)); + } + + static bool MethodHasOverloadForApiOptions(MethodInfo methodInfo, ParameterInfo[] expected) + { + var actual = methodInfo.GetParameters(); + + if (actual.Length != expected.Length + 1) + { + return false; + } + + for (var i = 0; i < expected.Length; i++) + { + var a = actual.ElementAt(i); + var e = expected.ElementAt(i); + + if (a.Name != e.Name) + { + return false; + } + + if (a.ParameterType != e.ParameterType) + { + return false; + } + } + + var lastParameter = actual.Last(); + + return lastParameter.Name == "options" + && lastParameter.ParameterType == typeof(ApiOptions); + } + + public static IEnumerable GetClientInterfaces() + { + return typeof(IEventsClient).Assembly.ExportedTypes + .Where(TypeExtensions.IsClientInterface) + .Where(type => type != typeof(IStatisticsClient)) + .Select(type => new[] { type }); + } + } +} diff --git a/Octokit.Tests.Integration/Clients/ReleasesClientTests.cs b/Octokit.Tests.Integration/Clients/ReleasesClientTests.cs index 0dd4df70..34892219 100644 --- a/Octokit.Tests.Integration/Clients/ReleasesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/ReleasesClientTests.cs @@ -75,6 +75,83 @@ public class ReleasesClientTests } } + public class TheGetAllMethod + { + readonly IReleasesClient _releaseClient; + const string owner = "octokit"; + const string name = "octokit.net"; + + public TheGetAllMethod() + { + var github = Helper.GetAuthenticatedClient(); + _releaseClient = github.Repository.Release; + } + + [IntegrationTest] + public async Task ReturnsReleases() + { + var releases = await _releaseClient.GetAll(owner, name); + + Assert.NotEmpty(releases); + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfReleasesWithoutStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; + + var releases = await _releaseClient.GetAll(owner, name, options); + + Assert.Equal(5, releases.Count); + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfReleasesWithStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var releases = await _releaseClient.GetAll(owner, name, options); + + Assert.Equal(5, releases.Count); + } + + [IntegrationTest] + public async Task ReturnsDistinctResultsBasedOnStartPage() + { + var startOptions = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; + + var firstPage = await _releaseClient.GetAll(owner, name, startOptions); + + var skipStartOptions = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var secondPage = await _releaseClient.GetAll(owner, name, skipStartOptions); + + Assert.NotEqual(firstPage[0].Id, secondPage[0].Id); + Assert.NotEqual(firstPage[1].Id, secondPage[1].Id); + Assert.NotEqual(firstPage[2].Id, secondPage[2].Id); + Assert.NotEqual(firstPage[3].Id, secondPage[3].Id); + Assert.NotEqual(firstPage[4].Id, secondPage[4].Id); + } + } + public class TheEditMethod : IDisposable { private readonly IGitHubClient _github; diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index 93cf79b3..5b843b6e 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -139,6 +139,7 @@ + diff --git a/Octokit.Tests.Integration/Reactive/ObservableReleaseClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableReleaseClientTests.cs new file mode 100644 index 00000000..337f2894 --- /dev/null +++ b/Octokit.Tests.Integration/Reactive/ObservableReleaseClientTests.cs @@ -0,0 +1,88 @@ +using System.Reactive.Linq; +using System.Threading.Tasks; +using Octokit.Reactive; +using Xunit; + +namespace Octokit.Tests.Integration.Reactive +{ + public class ObservableReleaseClientTests + { + public class TheGetAllMethod + { + readonly ObservableReleasesClient _releaseClient; + const string owner = "octokit"; + const string name = "octokit.net"; + + public TheGetAllMethod() + { + var github = Helper.GetAuthenticatedClient(); + + _releaseClient = new ObservableReleasesClient(github); + } + + [IntegrationTest] + public async Task ReturnsReleases() + { + var releases = await _releaseClient.GetAll(owner, name).ToList(); + + Assert.NotEmpty(releases); + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfReleasesWithoutStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; + + var releases = await _releaseClient.GetAll(owner, name, options).ToList(); + + Assert.Equal(5, releases.Count); + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfReleasesWithStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var releases = await _releaseClient.GetAll(owner, name, options).ToList(); + + Assert.Equal(5, releases.Count); + } + + [IntegrationTest] + public async Task ReturnsDistinctResultsBasedOnStartPage() + { + var startOptions = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; + + var firstPage = await _releaseClient.GetAll(owner, name, startOptions).ToList(); + + var skipStartOptions = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var secondPage = await _releaseClient.GetAll(owner, name, skipStartOptions).ToList(); + + Assert.NotEqual(firstPage[0].Id, secondPage[0].Id); + Assert.NotEqual(firstPage[1].Id, secondPage[1].Id); + Assert.NotEqual(firstPage[2].Id, secondPage[2].Id); + Assert.NotEqual(firstPage[3].Id, secondPage[3].Id); + Assert.NotEqual(firstPage[4].Id, secondPage[4].Id); + } + } + } +} diff --git a/Octokit.Tests/Clients/AuthorizationsClientTests.cs b/Octokit.Tests/Clients/AuthorizationsClientTests.cs index af18967e..5aad42ef 100644 --- a/Octokit.Tests/Clients/AuthorizationsClientTests.cs +++ b/Octokit.Tests/Clients/AuthorizationsClientTests.cs @@ -35,7 +35,7 @@ namespace Octokit.Tests.Clients client.Received().GetAll( Arg.Is(u => u.ToString() == "authorizations"), - null); + Args.ApiOptions); } } diff --git a/Octokit.Tests/Clients/ReleasesClientTests.cs b/Octokit.Tests/Clients/ReleasesClientTests.cs index b78ec427..d82c0707 100644 --- a/Octokit.Tests/Clients/ReleasesClientTests.cs +++ b/Octokit.Tests/Clients/ReleasesClientTests.cs @@ -2,14 +2,13 @@ using System.IO; using System.Threading.Tasks; using NSubstitute; -using Octokit.Tests.Helpers; using Xunit; namespace Octokit.Tests.Clients { public class ReleasesClientTests { - public class TheGetReleasesMethod + public class TheGetAllMethod { [Fact] public void RequestsCorrectUrl() @@ -21,7 +20,8 @@ namespace Octokit.Tests.Clients client.Received().GetAll(Arg.Is(u => u.ToString() == "repos/fake/repo/releases"), null, - "application/vnd.github.v3"); + "application/vnd.github.v3", + Args.ApiOptions); } [Fact] diff --git a/Octokit.Tests/Helpers/Arg.cs b/Octokit.Tests/Helpers/Arg.cs index 391255fa..5408f72d 100644 --- a/Octokit.Tests/Helpers/Arg.cs +++ b/Octokit.Tests/Helpers/Arg.cs @@ -67,5 +67,10 @@ namespace Octokit.Tests { get { return Arg.Any(); } } + + public static ApiOptions ApiOptions + { + get { return Arg.Any(); } + } } } diff --git a/Octokit.Tests/Http/ApiConnectionTests.cs b/Octokit.Tests/Http/ApiConnectionTests.cs index f0dd7b6d..5f3b7d22 100644 --- a/Octokit.Tests/Http/ApiConnectionTests.cs +++ b/Octokit.Tests/Http/ApiConnectionTests.cs @@ -90,13 +90,13 @@ namespace Octokit.Tests.Http new Response(), new List { new object(), new object() }); var connection = Substitute.For(); - connection.Get>(Args.Uri, null, null).Returns(Task.FromResult(response)); + connection.Get>(Args.Uri, Args.EmptyDictionary, null).Returns(Task.FromResult(response)); var apiConnection = new ApiConnection(connection); var data = await apiConnection.GetAll(getAllUri); Assert.Equal(2, data.Count); - connection.Received().Get>(getAllUri, null, null); + connection.Received().Get>(getAllUri, Args.EmptyDictionary, null); } [Fact] diff --git a/Octokit.Tests/Models/ReadOnlyPagedCollectionTests.cs b/Octokit.Tests/Models/ReadOnlyPagedCollectionTests.cs index 6fa1e4c2..908ff7e7 100644 --- a/Octokit.Tests/Models/ReadOnlyPagedCollectionTests.cs +++ b/Octokit.Tests/Models/ReadOnlyPagedCollectionTests.cs @@ -33,6 +33,70 @@ namespace Octokit.Tests.Models Assert.NotNull(nextPage); Assert.Equal(2, nextPage.Count); } + + [Fact] + public async Task WhenNoInformationSetReturnsNull() + { + var nextPageUrl = new Uri("https://example.com/page/2"); + var nextPageResponse = Task.Factory.StartNew>>(() => + new ApiResponse>(new Response(), new List { new object(), new object() })); + + var links = new Dictionary(); + var scopes = new List(); + var httpResponse = Substitute.For(); + httpResponse.ApiInfo.Returns(new ApiInfo(links, scopes, scopes, "etag", new RateLimit(new Dictionary()))); + + var response = new ApiResponse>(httpResponse, new List()); + var connection = Substitute.For(); + + connection.Get>(nextPageUrl, null, null).Returns(nextPageResponse); + + var pagedCollection = new ReadOnlyPagedCollection( + response, + nextPageUri => connection.Get>(nextPageUrl, null, null)); + + var nextPage = await pagedCollection.GetNextPage(); + + Assert.Null(nextPage); + } + + [Fact] + public async Task WhenInlineFuncKillsPaginationReturnNull() + { + var nextPageUrl = new Uri("https://example.com/page/2"); + var nextPageResponse = Task.Factory.StartNew>>(() => + new ApiResponse>(new Response(), new List { new object(), new object() })); + + var links = new Dictionary { { "next", nextPageUrl } }; + var scopes = new List(); + var httpResponse = Substitute.For(); + httpResponse.ApiInfo.Returns(new ApiInfo(links, scopes, scopes, "etag", new RateLimit(new Dictionary()))); + + var response = new ApiResponse>(httpResponse, new List()); + var connection = Substitute.For(); + + connection.Get>(nextPageUrl, null, null).Returns(nextPageResponse); + + var pageCount = 1; + + var pagedCollection = new ReadOnlyPagedCollection( + response, + nextPageUri => + { + if (pageCount > 1) + { + return null; + } + pageCount++; + return connection.Get>(nextPageUrl, null, null); + }); + + var first = await pagedCollection.GetNextPage(); + var second = await pagedCollection.GetNextPage(); + + Assert.NotNull(first); + Assert.Null(second); + } } } } diff --git a/Octokit/Clients/AuthorizationsClient.cs b/Octokit/Clients/AuthorizationsClient.cs index 440918a5..84b41b78 100644 --- a/Octokit/Clients/AuthorizationsClient.cs +++ b/Octokit/Clients/AuthorizationsClient.cs @@ -36,7 +36,7 @@ namespace Octokit /// A list of s. public Task> GetAll() { - return ApiConnection.GetAll(ApiUrls.Authorizations(), null); + return ApiConnection.GetAll(ApiUrls.Authorizations(), ApiOptions.None); } /// diff --git a/Octokit/Clients/IReleasesClient.cs b/Octokit/Clients/IReleasesClient.cs index d04b0cd3..435f67ae 100644 --- a/Octokit/Clients/IReleasesClient.cs +++ b/Octokit/Clients/IReleasesClient.cs @@ -26,6 +26,19 @@ namespace Octokit /// The list of s for the specified repository. Task> GetAll(string owner, string name); + /// + /// Gets all s for the specified repository. + /// + /// + /// See the API documentation for more information. + /// + /// The repository's owner + /// The repository's name + /// Options for changing the API response + /// Thrown when a general API error occurs. + /// The list of s for the specified repository. + Task> GetAll(string owner, string name, ApiOptions options); + /// /// Gets a single for the specified repository. /// diff --git a/Octokit/Clients/ReleasesClient.cs b/Octokit/Clients/ReleasesClient.cs index fae7bc4d..7fd95fea 100644 --- a/Octokit/Clients/ReleasesClient.cs +++ b/Octokit/Clients/ReleasesClient.cs @@ -36,8 +36,28 @@ namespace Octokit Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); Ensure.ArgumentNotNullOrEmptyString(name, "repository"); + return GetAll(owner, name, ApiOptions.None); + } + + /// + /// Gets all s for the specified repository. + /// + /// + /// See the API documentation for more information. + /// + /// The repository's owner + /// The repository's name + /// Options for changing the API response + /// Thrown when a general API error occurs. + /// The list of s for the specified repository. + public Task> GetAll(string owner, string name, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "repository"); + Ensure.ArgumentNotNull(options, "options"); + var endpoint = ApiUrls.Releases(owner, name); - return ApiConnection.GetAll(endpoint, null, AcceptHeaders.StableVersion); + return ApiConnection.GetAll(endpoint, null, AcceptHeaders.StableVersion, options); } /// diff --git a/Octokit/Helpers/ApiExtensions.cs b/Octokit/Helpers/ApiExtensions.cs index 5a3c9a15..81b1ab8e 100644 --- a/Octokit/Helpers/ApiExtensions.cs +++ b/Octokit/Helpers/ApiExtensions.cs @@ -26,7 +26,7 @@ namespace Octokit Ensure.ArgumentNotNull(connection, "connection"); Ensure.ArgumentNotNull(uri, "uri"); - return connection.GetAll(uri, null); + return connection.GetAll(uri, ApiOptions.None); } /// diff --git a/Octokit/Helpers/Pagination.cs b/Octokit/Helpers/Pagination.cs new file mode 100644 index 00000000..1acf39ea --- /dev/null +++ b/Octokit/Helpers/Pagination.cs @@ -0,0 +1,76 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; + +namespace Octokit +{ + internal static class Pagination + { + internal static IDictionary Setup(IDictionary parameters, ApiOptions options) + { + parameters = parameters ?? new Dictionary(); + + if (options.PageSize.HasValue) + { + parameters.Add("per_page", options.PageSize.Value.ToString(CultureInfo.InvariantCulture)); + } + + if (options.StartPage.HasValue) + { + parameters.Add("page", options.StartPage.Value.ToString(CultureInfo.InvariantCulture)); + } + + return parameters; + } + + internal static bool ShouldContinue( + Uri uri, + ApiOptions options) + { + if (uri == null) + { + return false; + } + + if (uri.Query.Contains("page=") && options.PageCount.HasValue) + { + var allValues = ToQueryStringDictionary(uri); + + string pageValue; + if (allValues.TryGetValue("page", out pageValue)) + { + var startPage = options.StartPage ?? 1; + var pageCount = options.PageCount.Value; + + var endPage = startPage + pageCount; + if (pageValue.Equals(endPage.ToString(CultureInfo.InvariantCulture), StringComparison.OrdinalIgnoreCase)) + { + return false; + } + } + } + + return true; + } + + static Dictionary ToQueryStringDictionary(Uri uri) + { + return uri.Query.Split('&') + .Select(keyValue => + { + var indexOf = keyValue.IndexOf('='); + if (indexOf > 0) + { + var key = keyValue.Substring(0, indexOf); + var value = keyValue.Substring(indexOf + 1); + return new KeyValuePair(key, value); + } + + //just a plain old value, return it + return new KeyValuePair(keyValue, null); + }) + .ToDictionary(x => x.Key, x => x.Value); + } + } +} diff --git a/Octokit/Http/ApiConnection.cs b/Octokit/Http/ApiConnection.cs index 33a77f6e..c1d2710a 100644 --- a/Octokit/Http/ApiConnection.cs +++ b/Octokit/Http/ApiConnection.cs @@ -115,7 +115,20 @@ namespace Octokit /// Thrown when an API error occurs. public Task> GetAll(Uri uri) { - return GetAll(uri, null, null); + return GetAll(uri, ApiOptions.None); + } + + /// + /// Gets all API resources in the list at the specified URI. + /// + /// Type of the API resource in the list. + /// URI of the API resource to get + /// Options for changing the API response + /// of the The API resources in the list. + /// Thrown when an API error occurs. + public Task> GetAll(Uri uri, ApiOptions options) + { + return GetAll(uri, null, null, options); } /// @@ -128,7 +141,21 @@ namespace Octokit /// Thrown when an API error occurs. public Task> GetAll(Uri uri, IDictionary parameters) { - return GetAll(uri, parameters, null); + return GetAll(uri, parameters, null, ApiOptions.None); + } + + /// + /// Gets all API resources in the list at the specified URI. + /// + /// Type of the API resource in the list. + /// URI of the API resource to get + /// Parameters to add to the API request + /// Options for changing the API response + /// of the The API resources in the list. + /// Thrown when an API error occurs. + public Task> GetAll(Uri uri, IDictionary parameters, ApiOptions options) + { + return GetAll(uri, parameters, null, options); } /// @@ -148,6 +175,17 @@ namespace Octokit .ConfigureAwait(false), uri); } + public Task> GetAll(Uri uri, IDictionary parameters, string accepts, ApiOptions options) + { + Ensure.ArgumentNotNull(uri, "uri"); + Ensure.ArgumentNotNull(options, "options"); + + parameters = Pagination.Setup(parameters, options); + + return _pagination.GetAllPages(async () => await GetPage(uri, parameters, accepts, options) + .ConfigureAwait(false), uri); + } + /// /// Creates a new API resource in the list at the specified URI. /// @@ -498,5 +536,30 @@ namespace Octokit response, nextPageUri => Connection.Get>(nextPageUri, parameters, accepts)); } + + async Task> GetPage( + Uri uri, + IDictionary parameters, + string accepts, + ApiOptions options) + { + Ensure.ArgumentNotNull(uri, "uri"); + + var connection = Connection; + + var response = await connection.Get>(uri, parameters, accepts).ConfigureAwait(false); + return new ReadOnlyPagedCollection( + response, + nextPageUri => + { + var shouldContinue = Pagination.ShouldContinue( + nextPageUri, + options); + + return shouldContinue + ? connection.Get>(nextPageUri, parameters, accepts) + : null; + }); + } } } diff --git a/Octokit/Http/IApiConnection.cs b/Octokit/Http/IApiConnection.cs index 5805df83..4984870b 100644 --- a/Octokit/Http/IApiConnection.cs +++ b/Octokit/Http/IApiConnection.cs @@ -71,6 +71,16 @@ namespace Octokit /// Thrown when an API error occurs. Task> GetAll(Uri uri); + /// + /// Gets all API resources in the list at the specified URI. + /// + /// Type of the API resource in the list. + /// URI of the API resource to get + /// Options for changing the API response + /// of the The API resources in the list. + /// Thrown when an API error occurs. + Task> GetAll(Uri uri, ApiOptions options); + /// /// Gets all API resources in the list at the specified URI. /// @@ -81,6 +91,17 @@ namespace Octokit /// Thrown when an API error occurs. Task> GetAll(Uri uri, IDictionary parameters); + /// + /// Gets all API resources in the list at the specified URI. + /// + /// Type of the API resource in the list. + /// URI of the API resource to get + /// Parameters to add to the API request + /// Options for changing the API response + /// of the The API resources in the list. + /// Thrown when an API error occurs. + Task> GetAll(Uri uri, IDictionary parameters, ApiOptions options); + /// /// Gets all API resources in the list at the specified URI. /// @@ -92,6 +113,18 @@ namespace Octokit /// Thrown when an API error occurs. Task> GetAll(Uri uri, IDictionary parameters, string accepts); + /// + /// Gets all API resources in the list at the specified URI. + /// + /// Type of the API resource in the list. + /// URI of the API resource to get + /// Parameters to add to the API request + /// Accept header to use for the API request + /// Options for changing the API response + /// of the The API resources in the list. + /// Thrown when an API error occurs. + Task> GetAll(Uri uri, IDictionary parameters, string accepts, ApiOptions options); + /// /// Creates a new API resource in the list at the specified URI. /// diff --git a/Octokit/Http/ReadOnlyPagedCollection.cs b/Octokit/Http/ReadOnlyPagedCollection.cs index 9e642824..520f107b 100644 --- a/Octokit/Http/ReadOnlyPagedCollection.cs +++ b/Octokit/Http/ReadOnlyPagedCollection.cs @@ -28,7 +28,14 @@ namespace Octokit.Internal var nextPageUrl = _info.GetNextPageUrl(); if (nextPageUrl == null) return null; - var response = await _nextPageFunc(nextPageUrl).ConfigureAwait(false); + var maybeTask = _nextPageFunc(nextPageUrl); + + if (maybeTask == null) + { + return null; + } + + var response = await maybeTask.ConfigureAwait(false); return new ReadOnlyPagedCollection(response, _nextPageFunc); } } diff --git a/Octokit/Models/Request/ApiOptions.cs b/Octokit/Models/Request/ApiOptions.cs new file mode 100644 index 00000000..ef149633 --- /dev/null +++ b/Octokit/Models/Request/ApiOptions.cs @@ -0,0 +1,62 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; + +namespace Octokit +{ + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class ApiOptions + { + public static ApiOptions None + { + get { return new ApiOptions(); } + } + + /// + /// Specify the start page for pagination actions + /// + /// + /// Page numbering is 1-based on the server + /// + public int? StartPage { get; set; } + + /// + /// Specify the number of pages to return + /// + public int? PageCount { get; set; } + + /// + /// Specify the number of results to return for each page + /// + /// + /// Results returned may be less than this total if you reach the final page of results + /// + public int? PageSize { get; set; } + + internal string DebuggerDisplay + { + get + { + var values = new List(); + + if (StartPage.HasValue) + { + values.Add("StartPage: " + StartPage.Value); + } + + if (PageCount.HasValue) + { + values.Add("PageCount: " + PageCount.Value); + } + + if (PageSize.HasValue) + { + values.Add("PageSize: " + PageSize.Value); + } + + return String.Join(", ", values); + } + } + } + +} diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index eca26aea..abbfe348 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -447,6 +447,8 @@ + + diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 5b72f2a8..60896d14 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -456,6 +456,8 @@ + + @@ -467,4 +469,4 @@ - \ No newline at end of file + diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index f02ec307..41d2a7e0 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -452,6 +452,8 @@ + + @@ -464,4 +466,4 @@ - \ No newline at end of file + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index e4d48b60..0f950019 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -444,6 +444,8 @@ + + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index da554e94..3cfdc341 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -451,6 +451,8 @@ + + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index b30d6435..f6004fb9 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -103,6 +103,7 @@ + @@ -112,6 +113,7 @@ + diff --git a/docs/extensibility.md b/docs/extensibility.md new file mode 100644 index 00000000..abab1675 --- /dev/null +++ b/docs/extensibility.md @@ -0,0 +1,25 @@ +# Extensibility + +Octokit.net has been designed to be easy to get started, but there are options +available to tweak the default behaviour once you know the basics. + +## Pagination + +The GitHub API supports paging results whenever collections are returned. + +By default, Octokit.net will fetch the entire set of data. Any method prefixed with +`GetAll*` now has an overload which accepts an `ApiOptions` parameter. + +```csharp +var options = new ApiOptions(); +var repositories = await client.Repository.GetAllForCurrent(options); +``` + +`ApiOptions` has a number of properties: + + - `PageCount` - return a set number of pages + - `PageSize` - change the number of results to return per page + - `StartPage` - start results from a given page + +These parameters can be used in any sort of group. If you don't specify a +`PageSize` the default page size is 30. diff --git a/mkdocs.yml b/mkdocs.yml index 713d7cbf..3ac0cf66 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -29,6 +29,7 @@ pages: - 'Exploring Pull Requests' : 'demos/exploring-pull-requests.md' - Advanced: + - 'API Options': 'extensibility.md' - 'Debugging from Source': 'debugging-source.md' - 'OAuth Flow': 'oauth-flow.md' - 'HttpClient': 'http-client.md'