From 65bb8d57d7a2fb53234a02f803ded95abd2294c1 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Tue, 25 Feb 2020 18:24:42 -0400 Subject: [PATCH] Allow fully qualified or short references to be used to generate "reference" Uri (#2110) Co-authored-by: Brian --- .../Clients/ObservableReferencesClient.cs | 26 ++++- .../Clients/ReferencesClientTests.cs | 43 +++++++ .../Clients/ReferencesClientTests.cs | 68 ++++++++++- Octokit/Clients/IReferencesClient.cs | 89 ++++++++++++--- Octokit/Clients/ReferencesClient.cs | 108 +++++++++++++++--- 5 files changed, 297 insertions(+), 37 deletions(-) diff --git a/Octokit.Reactive/Clients/ObservableReferencesClient.cs b/Octokit.Reactive/Clients/ObservableReferencesClient.cs index b5353bd2..21fcba50 100644 --- a/Octokit.Reactive/Clients/ObservableReferencesClient.cs +++ b/Octokit.Reactive/Clients/ObservableReferencesClient.cs @@ -146,7 +146,12 @@ namespace Octokit.Reactive /// The name of the repository /// The sub-namespace to get references for /// Options for changing the API response - /// + /// + /// The subNamespace parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public IObservable GetAllForSubNamespace(string owner, string name, string subNamespace, ApiOptions options) { Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); @@ -154,6 +159,11 @@ namespace Octokit.Reactive Ensure.ArgumentNotNullOrEmptyString(subNamespace, nameof(subNamespace)); Ensure.ArgumentNotNull(options, nameof(options)); + if (subNamespace.StartsWith("refs/")) + { + subNamespace = subNamespace.Replace("refs/", string.Empty); + } + return _connection.GetAndFlattenAllPages(ApiUrls.Reference(owner, name, subNamespace), options); } @@ -180,12 +190,22 @@ namespace Octokit.Reactive /// The Id of the repository /// The sub-namespace to get references for /// Options for changing the API response - /// + /// + /// The subNamespace parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public IObservable GetAllForSubNamespace(long repositoryId, string subNamespace, ApiOptions options) { Ensure.ArgumentNotNullOrEmptyString(subNamespace, nameof(subNamespace)); Ensure.ArgumentNotNull(options, nameof(options)); + if (subNamespace.StartsWith("refs/")) + { + subNamespace = subNamespace.Replace("refs/", string.Empty); + } + return _connection.GetAndFlattenAllPages(ApiUrls.Reference(repositoryId, subNamespace), options); } @@ -298,4 +318,4 @@ namespace Octokit.Reactive return _reference.Delete(repositoryId, reference).ToObservable(); } } -} \ No newline at end of file +} diff --git a/Octokit.Tests.Integration/Clients/ReferencesClientTests.cs b/Octokit.Tests.Integration/Clients/ReferencesClientTests.cs index ff6f930a..9f2c62bd 100644 --- a/Octokit.Tests.Integration/Clients/ReferencesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/ReferencesClientTests.cs @@ -179,6 +179,13 @@ public class ReferencesClientTests : IDisposable Assert.NotEmpty(list); } + [IntegrationTest] + public async Task CanGetListOfReferencesInNamespaceWithRefsIncluded() + { + var list = await _fixture.GetAllForSubNamespace("octokit", "octokit.net", "refs/heads"); + Assert.NotEmpty(list); + } + [IntegrationTest] public async Task ReturnsCorrectCountOfReferencesInNamespaceWithStart() { @@ -503,6 +510,42 @@ public class ReferencesClientTests : IDisposable Assert.Empty(all.Where(r => r.Ref == "heads/develop")); } + [IntegrationTest] + public async Task CanDeleteAReferenceUsingRefs() + { + var blob = new NewBlob + { + Content = "Hello World!", + Encoding = EncodingType.Utf8 + }; + var blobResult = await _github.Git.Blob.Create(_context.RepositoryOwner, _context.RepositoryName, blob); + + var newTree = new NewTree(); + newTree.Tree.Add(new NewTreeItem + { + Mode = FileMode.File, + Type = TreeType.Blob, + Path = "README.md", + Sha = blobResult.Sha + }); + + var treeResult = await _github.Git.Tree.Create(_context.RepositoryOwner, _context.RepositoryName, newTree); + + var newCommit = new NewCommit("This is a new commit", treeResult.Sha); + + var commitResult = await _github.Git.Commit.Create(_context.RepositoryOwner, _context.RepositoryName, newCommit); + + var newReference = new NewReference("heads/develop", commitResult.Sha); + + await _fixture.Create(_context.RepositoryOwner, _context.RepositoryName, newReference); + await _fixture.Delete(_context.RepositoryOwner, _context.RepositoryName, "refs/heads/develop"); + + var all = await _fixture.GetAll(_context.RepositoryOwner, _context.RepositoryName); + + Assert.Empty(all.Where(r => r.Ref == "heads/develop")); + } + + [IntegrationTest] public async Task CanDeleteAReferenceWithRepositoryId() { diff --git a/Octokit.Tests/Clients/ReferencesClientTests.cs b/Octokit.Tests/Clients/ReferencesClientTests.cs index 782004f6..1756f3de 100644 --- a/Octokit.Tests/Clients/ReferencesClientTests.cs +++ b/Octokit.Tests/Clients/ReferencesClientTests.cs @@ -47,6 +47,17 @@ namespace Octokit.Tests.Clients connection.Received().Get(Arg.Is(u => u.ToString() == "repos/owner/repo/git/refs/heads/develop")); } + [Fact] + public async Task RequestsCorrectUrlWithRef() + { + var connection = Substitute.For(); + var client = new ReferencesClient(connection); + + await client.Get("owner", "repo", "refs/heads/develop"); + + connection.Received().Get(Arg.Is(u => u.ToString() == "repos/owner/repo/git/refs/heads/develop")); + } + [Fact] public async Task RequestsCorrectUrlWithRepositoryId() { @@ -57,6 +68,17 @@ namespace Octokit.Tests.Clients connection.Received().Get(Arg.Is(u => u.ToString() == "repositories/1/git/refs/heads/develop")); } + + [Fact] + public async Task RequestsCorrectUrlWithRepositoryIdAndRefs() + { + var connection = Substitute.For(); + var client = new ReferencesClient(connection); + + await client.Get(1, "refs/heads/develop"); + + connection.Received().Get(Arg.Is(u => u.ToString() == "repositories/1/git/refs/heads/develop")); + } } public class TheGetAllMethod @@ -150,6 +172,17 @@ namespace Octokit.Tests.Clients connection.Received().GetAll(Arg.Is(u => u.ToString() == "repos/owner/repo/git/refs/heads"), Args.ApiOptions); } + [Fact] + public async Task RequestsCorrectUrlWithRef() + { + var connection = Substitute.For(); + var client = new ReferencesClient(connection); + + await client.GetAllForSubNamespace("owner", "repo", "refs/heads"); + + connection.Received().GetAll(Arg.Is(u => u.ToString() == "repos/owner/repo/git/refs/heads"), Args.ApiOptions); + } + [Fact] public async Task RequestsCorrectUrlWithRepositoryId() { @@ -160,6 +193,17 @@ namespace Octokit.Tests.Clients connection.Received().GetAll(Arg.Is(u => u.ToString() == "repositories/1/git/refs/heads"), Args.ApiOptions); } + + [Fact] + public async Task RequestsCorrectUrlWithRepositoryIdAndRefs() + { + var connection = Substitute.For(); + var client = new ReferencesClient(connection); + + await client.GetAllForSubNamespace(1, "refs/heads"); + + connection.Received().GetAll(Arg.Is(u => u.ToString() == "repositories/1/git/refs/heads"), Args.ApiOptions); + } } public class TheCreateMethod @@ -282,6 +326,17 @@ namespace Octokit.Tests.Clients connection.Received().Delete(Arg.Is(u => u.ToString() == "repos/owner/repo/git/refs/heads/develop")); } + [Fact] + public async Task RequestsCorrectUrlWithRefs() + { + var connection = Substitute.For(); + var client = new ReferencesClient(connection); + + await client.Delete("owner", "repo", "refs/heads/develop"); + + connection.Received().Delete(Arg.Is(u => u.ToString() == "repos/owner/repo/git/refs/heads/develop")); + } + [Fact] public async Task RequestsCorrectUrlWithRepositoryId() { @@ -292,6 +347,17 @@ namespace Octokit.Tests.Clients connection.Received().Delete(Arg.Is(u => u.ToString() == "repositories/1/git/refs/heads/develop")); } + + [Fact] + public async Task RequestsCorrectUrlWithRepositoryIdAndRefs() + { + var connection = Substitute.For(); + var client = new ReferencesClient(connection); + + await client.Delete(1, "refs/heads/develop"); + + connection.Received().Delete(Arg.Is(u => u.ToString() == "repositories/1/git/refs/heads/develop")); + } } } -} \ No newline at end of file +} diff --git a/Octokit/Clients/IReferencesClient.cs b/Octokit/Clients/IReferencesClient.cs index 04122e2f..23369c07 100644 --- a/Octokit/Clients/IReferencesClient.cs +++ b/Octokit/Clients/IReferencesClient.cs @@ -20,8 +20,13 @@ namespace Octokit /// /// The owner of the repository /// The name of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" - /// + /// The reference name + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get", Justification = "Method makes a network request")] Task Get(string owner, string name, string reference); @@ -33,8 +38,13 @@ namespace Octokit /// http://developer.github.com/v3/git/refs/#get-a-reference /// /// The Id of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" - /// + /// The reference name + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get", Justification = "Method makes a network request")] Task Get(long repositoryId, string reference); @@ -92,7 +102,12 @@ namespace Octokit /// The owner of the repository /// The name of the repository /// The sub-namespace to get references for - /// + /// + /// The subNamespace parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task> GetAllForSubNamespace(string owner, string name, string subNamespace); /// @@ -105,7 +120,12 @@ namespace Octokit /// The name of the repository /// The sub-namespace to get references for /// Options for changing the API response - /// + /// + /// The subNamespace parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task> GetAllForSubNamespace(string owner, string name, string subNamespace, ApiOptions options); /// @@ -116,7 +136,12 @@ namespace Octokit /// /// The Id of the repository /// The sub-namespace to get references for - /// + /// + /// The subNamespace parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task> GetAllForSubNamespace(long repositoryId, string subNamespace); /// @@ -128,7 +153,12 @@ namespace Octokit /// The Id of the repository /// The sub-namespace to get references for /// Options for changing the API response - /// + /// + /// The subNamespace parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task> GetAllForSubNamespace(long repositoryId, string subNamespace, ApiOptions options); /// @@ -140,7 +170,12 @@ namespace Octokit /// The owner of the repository /// The name of the repository /// The reference to create - /// + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task Create(string owner, string name, NewReference reference); /// @@ -162,9 +197,14 @@ namespace Octokit /// /// The owner of the repository /// The name of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" + /// The reference name /// The updated reference data - /// + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task Update(string owner, string name, string reference, ReferenceUpdate referenceUpdate); /// @@ -174,9 +214,14 @@ namespace Octokit /// http://developer.github.com/v3/git/refs/#update-a-reference /// /// The Id of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" + /// The reference name /// The updated reference data - /// + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task Update(long repositoryId, string reference, ReferenceUpdate referenceUpdate); /// @@ -187,8 +232,13 @@ namespace Octokit /// /// The owner of the repository /// The name of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" - /// + /// The reference name + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task Delete(string owner, string name, string reference); /// @@ -198,8 +248,13 @@ namespace Octokit /// http://developer.github.com/v3/git/refs/#delete-a-reference /// /// The Id of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" - /// + /// The reference name + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// Task Delete(long repositoryId, string reference); } } diff --git a/Octokit/Clients/ReferencesClient.cs b/Octokit/Clients/ReferencesClient.cs index 1b5fdb3f..7d0dc87f 100644 --- a/Octokit/Clients/ReferencesClient.cs +++ b/Octokit/Clients/ReferencesClient.cs @@ -28,14 +28,24 @@ namespace Octokit /// /// The owner of the repository /// The name of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" - /// + /// The reference name + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public Task Get(string owner, string name, string reference) { Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); Ensure.ArgumentNotNullOrEmptyString(reference, nameof(reference)); + if (reference.StartsWith("refs/")) + { + reference = reference.Replace("refs/", string.Empty); + } + return ApiConnection.Get(ApiUrls.Reference(owner, name, reference)); } @@ -46,12 +56,22 @@ namespace Octokit /// http://developer.github.com/v3/git/refs/#get-a-reference /// /// The Id of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" - /// + /// The reference name + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public Task Get(long repositoryId, string reference) { Ensure.ArgumentNotNullOrEmptyString(reference, nameof(reference)); + if (reference.StartsWith("refs/")) + { + reference = reference.Replace("refs/", string.Empty); + } + return ApiConnection.Get(ApiUrls.Reference(repositoryId, reference)); } @@ -142,7 +162,12 @@ namespace Octokit /// The name of the repository /// The sub-namespace to get references for /// Options for changing the API response - /// + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public Task> GetAllForSubNamespace(string owner, string name, string subNamespace, ApiOptions options) { Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); @@ -150,7 +175,10 @@ namespace Octokit Ensure.ArgumentNotNullOrEmptyString(subNamespace, nameof(subNamespace)); Ensure.ArgumentNotNull(options, nameof(options)); - // TODO: Handle 404 when subNamespace cannot be found + if (subNamespace.StartsWith("refs/")) + { + subNamespace = subNamespace.Replace("refs/", string.Empty); + } return ApiConnection.GetAll(ApiUrls.Reference(owner, name, subNamespace), options); } @@ -178,13 +206,21 @@ namespace Octokit /// The Id of the repository /// The sub-namespace to get references for /// Options for changing the API response - /// + /// + /// The subNamespace parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public Task> GetAllForSubNamespace(long repositoryId, string subNamespace, ApiOptions options) { Ensure.ArgumentNotNullOrEmptyString(subNamespace, nameof(subNamespace)); Ensure.ArgumentNotNull(options, nameof(options)); - // TODO: Handle 404 when subNamespace cannot be found + if (subNamespace.StartsWith("refs/")) + { + subNamespace = subNamespace.Replace("refs/", string.Empty); + } return ApiConnection.GetAll(ApiUrls.Reference(repositoryId, subNamespace), options); } @@ -232,9 +268,14 @@ namespace Octokit /// /// The owner of the repository /// The name of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" + /// The reference name /// The updated reference data - /// + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public Task Update(string owner, string name, string reference, ReferenceUpdate referenceUpdate) { Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); @@ -242,6 +283,11 @@ namespace Octokit Ensure.ArgumentNotNullOrEmptyString(reference, nameof(reference)); Ensure.ArgumentNotNull(referenceUpdate, nameof(referenceUpdate)); + if (reference.StartsWith("refs/")) + { + reference = reference.Replace("refs/", string.Empty); + } + return ApiConnection.Patch(ApiUrls.Reference(owner, name, reference), referenceUpdate); } @@ -252,14 +298,24 @@ namespace Octokit /// http://developer.github.com/v3/git/refs/#update-a-reference /// /// The Id of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" + /// The reference name /// The updated reference data - /// + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public Task Update(long repositoryId, string reference, ReferenceUpdate referenceUpdate) { Ensure.ArgumentNotNullOrEmptyString(reference, nameof(reference)); Ensure.ArgumentNotNull(referenceUpdate, nameof(referenceUpdate)); + if (reference.StartsWith("refs/")) + { + reference = reference.Replace("refs/", string.Empty); + } + return ApiConnection.Patch(ApiUrls.Reference(repositoryId, reference), referenceUpdate); } @@ -271,14 +327,24 @@ namespace Octokit /// /// The owner of the repository /// The name of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" - /// + /// The reference name + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public Task Delete(string owner, string name, string reference) { Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner)); Ensure.ArgumentNotNullOrEmptyString(name, nameof(name)); Ensure.ArgumentNotNullOrEmptyString(reference, nameof(reference)); + if (reference.StartsWith("refs/")) + { + reference = reference.Replace("refs/", string.Empty); + } + return ApiConnection.Delete(ApiUrls.Reference(owner, name, reference)); } @@ -289,12 +355,22 @@ namespace Octokit /// http://developer.github.com/v3/git/refs/#delete-a-reference /// /// The Id of the repository - /// The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1" - /// + /// The reference name + /// + /// The reference parameter supports either the fully-qualified ref + /// (prefixed with "refs/", e.g. "refs/heads/master" or + /// "refs/tags/release-1") or the shortened form (omitting "refs/", e.g. + /// "heads/master" or "tags/release-1") + /// public Task Delete(long repositoryId, string reference) { Ensure.ArgumentNotNullOrEmptyString(reference, nameof(reference)); + if (reference.StartsWith("refs/")) + { + reference = reference.Replace("refs/", string.Empty); + } + return ApiConnection.Delete(ApiUrls.Reference(repositoryId, reference)); } }