From 7b1fa04b3878886d4496909501d5e8bcc56a0365 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 14 Apr 2023 22:41:24 +0400 Subject: [PATCH] [FEAT] Allow users to fetch all commits for two commits comparision (#2690) --- .../IObservableRepositoryCommitsClients.cs | 21 ++++++ .../ObservableRepositoryCommitsClients.cs | 35 +++++++++ .../Clients/RepositoryCommitsClientTests.cs | 24 ++++++- .../Clients/RepositoriesClientTests.cs | 17 +++++ Octokit/Clients/IRepositoryCommitsClient.cs | 21 ++++++ Octokit/Clients/RepositoryCommitsClient.cs | 72 ++++++++++++++++++- 6 files changed, 188 insertions(+), 2 deletions(-) diff --git a/Octokit.Reactive/Clients/IObservableRepositoryCommitsClients.cs b/Octokit.Reactive/Clients/IObservableRepositoryCommitsClients.cs index f9b2c95c..da58a215 100644 --- a/Octokit.Reactive/Clients/IObservableRepositoryCommitsClients.cs +++ b/Octokit.Reactive/Clients/IObservableRepositoryCommitsClients.cs @@ -62,6 +62,27 @@ namespace Octokit.Reactive [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "base")] IObservable Compare(long repositoryId, string @base, string head); + /// + /// Compare two references in a repository + /// + /// The owner of the repository + /// The name of the repository + /// The reference to use as the base commit + /// The reference to use as the head commit + /// Options for changing the API response + [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "base")] + IObservable Compare(string owner, string name, string @base, string head, ApiOptions options); + + /// + /// Compare two references in a repository + /// + /// The Id of the repository + /// The reference to use as the base commit + /// The reference to use as the head commit + /// Options for changing the API response + [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "base")] + IObservable Compare(long repositoryId, string @base, string head, ApiOptions options); + /// /// Gets all commits for a given repository /// diff --git a/Octokit.Reactive/Clients/ObservableRepositoryCommitsClients.cs b/Octokit.Reactive/Clients/ObservableRepositoryCommitsClients.cs index a2f6ad73..8e5e187b 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoryCommitsClients.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoryCommitsClients.cs @@ -110,6 +110,41 @@ namespace Octokit.Reactive return _commit.Compare(repositoryId, @base, head).ToObservable(); } + /// + /// Compare two references in a repository + /// + /// The owner of the repository + /// The name of the repository + /// The reference to use as the base commit + /// The reference to use as the head commit + /// Options for changing the API response + public IObservable Compare(string owner, string name, string @base, string head, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); + Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); + Ensure.ArgumentNotNullOrEmptyString(@base, "base"); + Ensure.ArgumentNotNullOrEmptyString(head, nameof(head)); + Ensure.ArgumentNotNull(options, nameof(options)); + + return _commit.Compare(owner, name, @base, head, options).ToObservable(); + } + + /// + /// Compare two references in a repository + /// + /// The Id of the repository + /// The reference to use as the base commit + /// The reference to use as the head commit + /// Options for changing the API response + public IObservable Compare(long repositoryId, string @base, string head, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(@base, "base"); + Ensure.ArgumentNotNullOrEmptyString(head, nameof(head)); + Ensure.ArgumentNotNull(options, nameof(options)); + + return _commit.Compare(repositoryId, @base, head, options).ToObservable(); + } + /// /// Gets all commits for a given repository /// diff --git a/Octokit.Tests.Integration/Clients/RepositoryCommitsClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryCommitsClientTests.cs index e1d769f8..80eb90ae 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryCommitsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryCommitsClientTests.cs @@ -297,6 +297,28 @@ public class RepositoryCommitsClientTests Assert.NotNull(sha1); } + + [IntegrationTest] + public async Task CanFetchAllCommitsInComparision() + { + const string @base = "8dcb1db0da7c86596bf1d63631bd335363c64b8c"; + const string head = "7349ecd6685c370ba84eb13f4c39f75f33"; + + var compareResultWithoutOptions = await _fixture.Compare(octokitNetRepositoryId, @base, head); + Assert.Equal(250, compareResultWithoutOptions.Commits.Count); + + var compareResult = await _fixture.Compare(octokitNetRepositoryId, @base, head, new ApiOptions { PageSize = 100 }); + Assert.Equal(534, compareResult.Commits.Count); + } + + [IntegrationTest] + public async Task CanCompareTheSameCommitWithApiOptions() + { + const string head = "7349ecd6685c370ba84eb13f4c39f75f33"; + + var compareResult = await _fixture.Compare(octokitNetRepositoryId, head, head, new ApiOptions { PageSize = 100 }); + Assert.Equal(0, compareResult.Commits.Count); + } } public class TestsWithNewRepository : IDisposable @@ -503,4 +525,4 @@ public class RepositoryCommitsClientTests _context.Dispose(); } } -} \ No newline at end of file +} diff --git a/Octokit.Tests/Clients/RepositoriesClientTests.cs b/Octokit.Tests/Clients/RepositoriesClientTests.cs index 9ed62ed4..a7b876da 100644 --- a/Octokit.Tests/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoriesClientTests.cs @@ -1186,6 +1186,23 @@ namespace Octokit.Tests.Clients await Assert.ThrowsAsync(() => client.Compare("owner", "repo", "base", null)); await Assert.ThrowsAsync(() => client.Compare("owner", "repo", "base", "")); + + + var options = new ApiOptions(); + await Assert.ThrowsAsync(() => client.Compare(null, "repo", "base", "head", options)); + await Assert.ThrowsAsync(() => client.Compare("", "repo", "base", "head", options)); + + await Assert.ThrowsAsync(() => client.Compare("owner", null, "base", "head", options)); + await Assert.ThrowsAsync(() => client.Compare("owner", "", "base", "head", options)); + + await Assert.ThrowsAsync(() => client.Compare("owner", "repo", null, "head", options)); + await Assert.ThrowsAsync(() => client.Compare("owner", "repo", "", "head", options)); + + await Assert.ThrowsAsync(() => client.Compare("owner", "repo", "base", null, options)); + await Assert.ThrowsAsync(() => client.Compare("owner", "repo", "base", "", options)); + + await Assert.ThrowsAsync(() => client.Compare("owner", "repo", "base", "head", null)); + await Assert.ThrowsAsync(() => client.Compare("owner", "repo", "base", "head", null)); } [Fact] diff --git a/Octokit/Clients/IRepositoryCommitsClient.cs b/Octokit/Clients/IRepositoryCommitsClient.cs index c91f8542..0a11a48a 100644 --- a/Octokit/Clients/IRepositoryCommitsClient.cs +++ b/Octokit/Clients/IRepositoryCommitsClient.cs @@ -63,6 +63,27 @@ namespace Octokit [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "base")] Task Compare(long repositoryId, string @base, string head); + /// + /// Compare two references in a repository + /// + /// The owner of the repository + /// The name of the repository + /// The reference to use as the base commit + /// The reference to use as the head commit + /// Options for changing the API response + [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "base")] + Task Compare(string owner, string name, string @base, string head, ApiOptions options); + + /// + /// Compare two references in a repository + /// + /// The Id of the repository + /// The reference to use as the base commit + /// The reference to use as the head commit + /// Options for changing the API response + [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "base")] + Task Compare(long repositoryId, string @base, string head, ApiOptions options); + /// /// Gets a single commit for a given repository /// diff --git a/Octokit/Clients/RepositoryCommitsClient.cs b/Octokit/Clients/RepositoryCommitsClient.cs index bf32ea4e..18f9da22 100644 --- a/Octokit/Clients/RepositoryCommitsClient.cs +++ b/Octokit/Clients/RepositoryCommitsClient.cs @@ -1,4 +1,6 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; namespace Octokit @@ -105,6 +107,74 @@ namespace Octokit return ApiConnection.Get(ApiUrls.RepoCompare(repositoryId, @base, head)); } + /// + /// Compare two references in a repository + /// + /// The owner of the repository + /// The name of the repository + /// The reference to use as the base commit + /// The reference to use as the head commit + /// Options for changing the API response + [ManualRoute("GET", "/repos/{owner}/{repo}/compare/{base}...{head}")] + public Task Compare(string owner, string name, string @base, string head, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); + Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); + Ensure.ArgumentNotNullOrEmptyString(@base, nameof(@base)); + Ensure.ArgumentNotNullOrEmptyString(head, nameof(head)); + Ensure.ArgumentNotNull(options, nameof(options)); + + return Compare(ApiUrls.RepoCompare(owner, name, @base, head), options); + } + + /// + /// Compare two references in a repository + /// + /// The Id of the repository + /// The reference to use as the base commit + /// The reference to use as the head commit + /// Options for changing the API response + [ManualRoute("GET", "/repositories/{id}/compare/{base}...{head}")] + public Task Compare(long repositoryId, string @base, string head, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(@base, nameof(@base)); + Ensure.ArgumentNotNullOrEmptyString(head, nameof(head)); + Ensure.ArgumentNotNull(options, nameof(options)); + + return Compare(ApiUrls.RepoCompare(repositoryId, @base, head), options); + } + + private async Task Compare(Uri uri, ApiOptions options) + { + var results = await ApiConnection.GetAll(uri, options); + if (results.Count == 1) return results[0]; + + var firstCompareResult = results[0]; + var commits = firstCompareResult.Commits.ToList(); + var files = firstCompareResult.Files.ToList(); + + foreach (var compareResult in results.Skip(1)) + { + commits.AddRange(compareResult.Commits ?? Array.Empty()); + files.AddRange(compareResult.Files ?? Array.Empty()); + } + + return new CompareResult( + firstCompareResult.Url, + firstCompareResult.HtmlUrl, + firstCompareResult.PermalinkUrl, + firstCompareResult.DiffUrl, + firstCompareResult.PatchUrl, + firstCompareResult.BaseCommit, + firstCompareResult.MergeBaseCommit, + firstCompareResult.Status, + firstCompareResult.AheadBy, + firstCompareResult.BehindBy, + firstCompareResult.TotalCommits, + commits, + files); + } + /// /// Gets a single commit for a given repository ///