Skip to content

Commit

Permalink
removed GetAllPublic overload due to paging issue and parameters clob…
Browse files Browse the repository at this point in the history
…bering
  • Loading branch information
shiftkey committed Apr 22, 2015
1 parent 082d3ed commit 26a8bf0
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 180 deletions.
12 changes: 0 additions & 12 deletions Octokit.Reactive/Clients/IObservableRepositoriesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,6 @@ public interface IObservableRepositoriesClient
Justification = "Makes a network request")]
IObservable<Repository> GetAllPublic();

/// <summary>
/// Retrieves every public <see cref="Repository"/> since the last repository seen.
/// </summary>
/// <remarks>
/// The default page size on GitHub.com is 30.
/// </remarks>
/// <param name="request">Search parameters of the last repository seen</param>
/// <returns>A <see cref="IReadOnlyPagedCollection{Repository}"/> of <see cref="Repository"/>.</returns>
[SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate",
Justification = "Makes a network request")]
IObservable<Repository> GetAllPublic(PublicRepositoryRequest request);

/// <summary>
/// Retrieves every <see cref="Repository"/> that belongs to the current user.
/// </summary>
Expand Down
15 changes: 0 additions & 15 deletions Octokit.Reactive/Clients/ObservableRepositoriesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,6 @@ public IObservable<Repository> GetAllPublic()
return _connection.GetAndFlattenAllPages<Repository>(ApiUrls.AllPublicRepositories());
}

/// <summary>
/// Retrieves every public <see cref="Repository"/> since the last repository seen.
/// </summary>
/// <remarks>
/// The default page size on GitHub.com is 30.
/// </remarks>
/// <param name="request">Search parameters of the last repository seen</param>
/// <returns>A <see cref="IReadOnlyPagedCollection{Repository}"/> of <see cref="Repository"/>.</returns>
public IObservable<Repository> GetAllPublic(PublicRepositoryRequest request)
{
Ensure.ArgumentNotNull(request, "request");

return _connection.GetAndFlattenAllPages<Repository>(ApiUrls.AllPublicRepositories(), request.ToParametersDictionary());
}

/// <summary>
/// Retrieves every <see cref="Repository"/> that belongs to the current user.
/// </summary>
Expand Down
28 changes: 14 additions & 14 deletions Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -553,20 +553,20 @@ public async Task ReturnsAllPublicRepositories()
Assert.True(repositories.Count > 80);
}

[IntegrationTest]
public async Task ReturnsAllPublicRepositoriesSinceLastSeen()
{
var github = Helper.GetAuthenticatedClient();

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);
}
//[IntegrationTest]
//public async Task ReturnsAllPublicRepositoriesSinceLastSeen()
//{
// var github = Helper.GetAuthenticatedClient();

// 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);
//}
}

public class TheGetAllForOrgMethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,5 @@ public async Task ReturnsSpecifiedRepository()
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);
}
}
}
}
31 changes: 0 additions & 31 deletions Octokit.Tests/Clients/RepositoriesClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -273,37 +273,6 @@ public void RequestsTheCorrectUrlAndReturnsRepositories()
}
}


public class TheGetAllPublicSinceMethod
{
[Fact]
public void RequestsTheCorrectUrl()
{
var connection = Substitute.For<IApiConnection>();
var client = new RepositoriesClient(connection);

client.GetAllPublic(new PublicRepositoryRequest(364));

connection.Received()
.GetAll<Repository>(Arg.Is<Uri>(u => u.ToString() == "/repositories"),
Arg.Any<Dictionary<string, string>>());
}

[Fact]
public void SendsTheCorrectParameter()
{
var connection = Substitute.For<IApiConnection>();
var client = new RepositoriesClient(connection);

client.GetAllPublic(new PublicRepositoryRequest(364));

connection.Received()
.GetAll<Repository>(Arg.Is<Uri>(u => u.ToString() == "/repositories"),
Arg.Is<Dictionary<string, string>>(d => d.Count == 1
&& d["since"] == "364"));
}
}

public class TheGetAllForCurrentMethod
{
[Fact]
Expand Down
59 changes: 0 additions & 59 deletions Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,65 +158,6 @@ public async Task StopsMakingNewRequestsWhenTakeIsFulfilled()
}
}

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<string, Uri> { { "next", secondPageUrl } };
IApiResponse<List<Repository>> firstPageResponse = new ApiResponse<List<Repository>>(
CreateResponseWithApiInfo(firstPageLinks),
new List<Repository>
{
new Repository(364),
new Repository(365),
new Repository(366)
});

var thirdPageUrl = new Uri("https://example.com/page/3");
var secondPageLinks = new Dictionary<string, Uri> { { "next", thirdPageUrl } };
IApiResponse<List<Repository>> secondPageResponse = new ApiResponse<List<Repository>>
(
CreateResponseWithApiInfo(secondPageLinks),
new List<Repository>
{
new Repository(367),
new Repository(368),
new Repository(369)
});

IApiResponse<List<Repository>> lastPageResponse = new ApiResponse<List<Repository>>(
new Response(),
new List<Repository>
{
new Repository(370)
});

var gitHubClient = Substitute.For<IGitHubClient>();
gitHubClient.Connection.Get<List<Repository>>(firstPageUrl,
Arg.Is<Dictionary<string, string>>(d => d.Count == 1
&& d["since"] == "364"), null)
.Returns(Task.FromResult(firstPageResponse));
gitHubClient.Connection.Get<List<Repository>>(secondPageUrl, null, null)
.Returns(Task.FromResult(secondPageResponse));
gitHubClient.Connection.Get<List<Repository>>(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<List<Repository>>(firstPageUrl,
Arg.Is<Dictionary<string, string>>(d=>d.Count == 1
&& d["since"] == "364"), null);
gitHubClient.Connection.Received(1).Get<List<Repository>>(secondPageUrl, null, null);
gitHubClient.Connection.Received(1).Get<List<Repository>>(thirdPageUrl, null, null);
}
}

public class TheGetAllBranchesMethod
{
[Fact]
Expand Down
14 changes: 0 additions & 14 deletions Octokit/Clients/IRepositoriesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,6 @@ public interface IRepositoriesClient
Justification = "Makes a network request")]
Task<IReadOnlyList<Repository>> GetAllPublic();


/// <summary>
/// Gets all public repositories since the integer ID of the last Repository that you've seen.
/// </summary>
/// <remarks>
/// See the <a href="https://developer.github.com/v3/repos/#list-all-public-repositories">API documentation</a> for more information.
/// The default page size on GitHub.com is 30.
/// </remarks>
/// <param name="request">Search parameters of the last repository seen</param>
/// <exception cref="AuthorizationException">Thrown if the client is not authenticated.</exception>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
/// <returns>A <see cref="IReadOnlyPagedCollection{Repository}"/> of <see cref="Repository"/>.</returns>
Task<IReadOnlyList<Repository>> GetAllPublic(PublicRepositoryRequest request);

/// <summary>
/// Gets all repositories owned by the current user.
/// </summary>
Expand Down
18 changes: 0 additions & 18 deletions Octokit/Clients/RepositoriesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -188,24 +188,6 @@ public Task<IReadOnlyList<Repository>> GetAllPublic()
return ApiConnection.GetAll<Repository>(ApiUrls.AllPublicRepositories());
}

/// <summary>
/// Gets all public repositories since the integer ID of the last Repository that you've seen.
/// </summary>
/// <remarks>
/// See the <a href="https://developer.github.com/v3/repos/#list-all-public-repositories">API documentation</a> for more information.
/// The default page size on GitHub.com is 30.
/// </remarks>
/// <param name="request">Search parameters of the last repository seen</param>
/// <exception cref="AuthorizationException">Thrown if the client is not authenticated.</exception>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
/// <returns>A <see cref="IReadOnlyPagedCollection{Repository}"/> of <see cref="Repository"/>.</returns>
public Task<IReadOnlyList<Repository>> GetAllPublic(PublicRepositoryRequest request)
{
Ensure.ArgumentNotNull(request, "request");

return ApiConnection.GetAll<Repository>(ApiUrls.AllPublicRepositories(), request.ToParametersDictionary());
}

/// <summary>
/// Gets all repositories owned by the current user.
/// </summary>
Expand Down

0 comments on commit 26a8bf0

Please sign in to comment.