Add pagination support to Migrations client (#1949)

* Add pagination to *MigrationsClient

* Add unit tests for *MigrationsClient

* Add integration tests for *MigrationsClient

* Fix the broken tests

Octokit.Reactive/Clients/IObservableMigrationsClient.cs

@@ -36,9 +36,22 @@ namespace Octokit.Reactive
         /// </remarks>
         /// <param name="org">The organization of which to list migrations.</param>
         /// <returns>List of most recent <see cref="Migration"/>s.</returns>
-        IObservable<List<Migration>> GetAll(
+        IObservable<Migration> GetAll(
             string org);
 
+        /// <summary>
+        /// Gets the list of the most recent migrations of the the organization.
+        /// </summary>
+        /// <remarks>
+        /// https://developer.github.com/v3/migration/migrations/#get-a-list-of-migrations
+        /// </remarks>
+        /// <param name="org">The organization of which to list migrations.</param>
+        /// <param name="options">Options for changing the API response</param>
+        /// <returns>List of most recent <see cref="Migration"/>s.</returns>
+        IObservable<Migration> GetAll(
+            string org,
+            ApiOptions options);
+
         /// <summary>
         /// Get the status of a migration.
         /// </summary>

Octokit.Reactive/Clients/ObservableMigrationsClient.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Reactive;
 using System.Reactive.Threading.Tasks;
+using Octokit.Reactive.Internal;
 
 namespace Octokit.Reactive
 {
@@ -15,6 +16,7 @@ namespace Octokit.Reactive
     public class ObservableMigrationsClient : IObservableMigrationsClient
     {
         private readonly IMigrationsClient _client;
+        private readonly IConnection _connection;
 
         /// <summary>
         /// Instantiates a GitHub Migrations API client.
@@ -25,6 +27,7 @@ namespace Octokit.Reactive
             Ensure.ArgumentNotNull(client, nameof(client));
 
             _client = client.Migration.Migrations;
+            _connection = client.Connection;
         }
 
         /// <summary>
@@ -50,9 +53,16 @@ namespace Octokit.Reactive
         /// </remarks>
         /// <param name="org">The organization of which to list migrations.</param>
         /// <returns>List of most recent <see cref="Migration"/>s.</returns>
-        public IObservable<List<Migration>> GetAll(string org)
+        public IObservable<Migration> GetAll(string org)
         {
-            return _client.GetAll(org).ToObservable();
+            return GetAll(org, ApiOptions.None);
+        }
+
+        public IObservable<Migration> GetAll(string org, ApiOptions options)
+        {
+            Ensure.ArgumentNotNull(options, nameof(options));
+            
+            return _connection.GetAndFlattenAllPages<Migration>(ApiUrls.EnterpriseMigrations(org), null, AcceptHeaders.MigrationsApiPreview, options);
         }
 
         /// <summary>

Octokit.Tests.Integration/Clients/MigrationsClientTests.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Security.Cryptography;
 using System.Threading;
 using System.Threading.Tasks;
 using Octokit;
@@ -68,6 +67,61 @@ public class MigrationsClientTests : IDisposable
         Assert.NotEqual(0, migrations.Count);
     }
 
+    [IntegrationTest]
+    public async Task ReturnsCorrectCountOfMigrationsWithoutStart()
+    {
+        var options = new ApiOptions
+        {
+            PageCount = 1,
+            PageSize = 1
+        };
+        
+        var migrations = await _gitHub.Migration.Migrations.GetAll(_orgName, options);
+
+        Assert.Equal(1, migrations.Count);
+    }
+    
+    [IntegrationTest]
+    public async Task ReturnsCorrectCountOfMigrationsWithStart()
+    {
+        var options = new ApiOptions
+        {
+            PageCount = 1,
+            PageSize = 1,
+            StartPage = 2
+        };
+        
+        var migrations = await _gitHub.Migration.Migrations.GetAll(_orgName, options);
+
+        Assert.Equal(1, migrations.Count);
+    }
+
+    [IntegrationTest]
+    public async Task ReturnsDistinctMigrationsBasedOnStartPage()
+    {
+        var startOptions = new ApiOptions
+        {
+            PageCount = 1,
+            PageSize = 1,
+            StartPage = 1
+        };
+        
+        var firstPage = await _gitHub.Migration.Migrations.GetAll(_orgName, startOptions);
+
+        var skipStartOptions = new ApiOptions
+        {
+            PageCount = 1,
+            PageSize = 1,
+            StartPage = 2
+        };
+        var secondPage = await _gitHub.Migration.Migrations.GetAll(_orgName, skipStartOptions);
+
+        Assert.Equal(1, firstPage.Count);
+        Assert.Equal(1, secondPage.Count);
+        Assert.NotEqual(firstPage[0].Id, secondPage[0].Id);
+        Assert.NotEqual(firstPage[0].Repositories, secondPage[0].Repositories);
+    }
+
     [IntegrationTest]
     public async Task CanGetMigration()
     {

Octokit.Tests/Clients/MigrationsClientTests.cs

@@ -55,10 +55,32 @@ namespace Octokit.Tests.Clients
 
                 client.GetAll("fake");
 
-                connection.Received().Get<List<Migration>>(
+                connection.Received().GetAll<Migration>(
                     Arg.Is<Uri>(u => u.ToString() == "orgs/fake/migrations"),
                     null,
-                    AcceptHeaders.MigrationsApiPreview);
+                    AcceptHeaders.MigrationsApiPreview,
+                    Args.ApiOptions);
+            }
+
+            [Fact]
+            public void RequestsCorrectUrlApiOptions()
+            {
+                var connection = Substitute.For<IApiConnection>();
+                var client = new MigrationsClient(connection);
+
+                var options = new ApiOptions
+                {
+                    PageCount = 1,
+                    PageSize = 1
+                };
+
+                client.GetAll("fake", options);
+
+                connection.Received().GetAll<Migration>(
+                    Arg.Is<Uri>(u => u.ToString() == "orgs/fake/migrations"),
+                    null,
+                    AcceptHeaders.MigrationsApiPreview,
+                    options);
             }
 
             [Fact]
@@ -68,6 +90,7 @@ namespace Octokit.Tests.Clients
                 var client = new MigrationsClient(connection);
 
                 await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAll(null));
+                await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetAll("fake", null));
                 await Assert.ThrowsAsync<ArgumentException>(() => client.GetAll(""));
             }
         }

Octokit.Tests/Reactive/ObservableMigrationsClientTests.cs

@@ -1,7 +1,9 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using NSubstitute;
 using Octokit.Reactive;
+using Octokit.Reactive.Internal;
 using Xunit;
 
 namespace Octokit.Tests.Reactive
@@ -46,7 +48,23 @@ namespace Octokit.Tests.Reactive
                 var client = new ObservableMigrationsClient(github);
 
                 client.GetAll("fake");
-                github.Migration.Migrations.Received(1).GetAll("fake");
+                github.Received().Migration.Migrations.GetAll("fake", Args.ApiOptions);
+            }
+
+            [Fact]
+            public void CallsIntoClientApiOptions()
+            {
+                var github = Substitute.For<IGitHubClient>();
+                var client = new ObservableMigrationsClient(github);
+                var options = new ApiOptions
+                {
+                    PageCount = 1,
+                    PageSize = 1
+                };
+
+                client.GetAll("fake", options);
+
+                github.Received().Migration.Migrations.GetAll("fake", options);
             }
         }
 

Octokit/Clients/IMigrationsClient.cs

@@ -35,10 +35,22 @@ namespace Octokit
         /// </remarks>
         /// <param name="org">The organization of which to list migrations.</param>
         /// <returns>List of most recent <see cref="Migration"/>s.</returns>
-        [ExcludeFromPaginationApiOptionsConventionTest("TODO: Implement pagination for this method")]
-        Task<List<Migration>> GetAll(
+        Task<IReadOnlyList<Migration>> GetAll(
             string org);
 
+        /// <summary>
+        /// Gets the list of the most recent migrations of the the organization.
+        /// </summary>
+        /// <remarks>
+        /// https://developer.github.com/v3/migration/migrations/#get-a-list-of-migrations
+        /// </remarks>
+        /// <param name="org">The organization of which to list migrations.</param>
+        /// <param name="options">Options for changing the API response</param>
+        /// <returns>List of most recent <see cref="Migration"/>s.</returns>
+        Task<IReadOnlyList<Migration>> GetAll(
+            string org,
+            ApiOptions options);
+
         /// <summary>
         /// Get the status of a migration
         /// </summary>

Octokit/Clients/MigrationsClient.cs

@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
 
 namespace Octokit
@@ -47,13 +48,28 @@ namespace Octokit
         /// </remarks>
         /// <param name="org">The organization of which to list migrations.</param>
         /// <returns>List of most recent <see cref="Migration"/>s.</returns>
-        public async Task<List<Migration>> GetAll(string org)
+        public async Task<IReadOnlyList<Migration>> GetAll(string org)
+        {
+            return await GetAll(org, ApiOptions.None);
+        }
+
+        /// <summary>
+        /// Gets the list of the most recent migrations of the the organization.
+        /// </summary>
+        /// <remarks>
+        /// https://developer.github.com/v3/migration/migrations/#get-a-list-of-migrations
+        /// </remarks>
+        /// <param name="org">The organization of which to list migrations.</param>
+        /// <param name="options">Options for changing the API response</param>
+        /// <returns>List of most recent <see cref="Migration"/>s.</returns>
+        public async Task<IReadOnlyList<Migration>> GetAll(string org, ApiOptions options)
         {
             Ensure.ArgumentNotNullOrEmptyString(org, nameof(org));
+            Ensure.ArgumentNotNull(options, nameof(options));
 
             var endpoint = ApiUrls.EnterpriseMigrations(org);
 
-            return await ApiConnection.Get<List<Migration>>(endpoint, null, AcceptHeaders.MigrationsApiPreview).ConfigureAwait(false);
+            return await ApiConnection.GetAll<Migration>(endpoint, null, AcceptHeaders.MigrationsApiPreview, options).ConfigureAwait(false);
         }
 
         /// <summary>