Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow fully qualified or short references to be used to generate "reference" Uri #2110

Merged
merged 4 commits into from
Feb 25, 2020
Merged

Allow fully qualified or short references to be used to generate "reference" Uri #2110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eae8607
Updated ApiUrls.Reference to remove 'refs' from referenceName paramet…
astrohart Jan 30, 2019
9c0bb2a
move fix up into clients and update docs and unit tests
shiftkey Feb 25, 2020
db13e84
add some integration tests that use refs
shiftkey Feb 25, 2020
308e8a2
keep internals simple for now
shiftkey Feb 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions Octokit.Reactive/Clients/ObservableReferencesClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,24 @@ public IObservable<Reference> GetAllForSubNamespace(string owner, string name, s
/// <param name="name">The name of the repository</param>
/// <param name="subNamespace">The sub-namespace to get references for</param>
/// <param name="options">Options for changing the API response</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
public IObservable<Reference> GetAllForSubNamespace(string owner, string name, string subNamespace, ApiOptions options)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Ensure.ArgumentNotNullOrEmptyString(subNamespace, nameof(subNamespace));
Ensure.ArgumentNotNull(options, nameof(options));

if (subNamespace.StartsWith("refs/"))
{
subNamespace = subNamespace.Replace("refs/", string.Empty);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up going with this approach to make the fixes more explicit, and I'd like to eventually do away with the ApiUrls shorthand because it makes correlating things to API routes harder...

}

return _connection.GetAndFlattenAllPages<Reference>(ApiUrls.Reference(owner, name, subNamespace), options);
}

Expand All @@ -180,12 +190,22 @@ public IObservable<Reference> GetAllForSubNamespace(long repositoryId, string su
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="subNamespace">The sub-namespace to get references for</param>
/// <param name="options">Options for changing the API response</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
public IObservable<Reference> 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<Reference>(ApiUrls.Reference(repositoryId, subNamespace), options);
}

Expand Down Expand Up @@ -298,4 +318,4 @@ public IObservable<Unit> Delete(long repositoryId, string reference)
return _reference.Delete(repositoryId, reference).ToObservable();
}
}
}
}
43 changes: 43 additions & 0 deletions Octokit.Tests.Integration/Clients/ReferencesClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ public async Task CanGetListOfReferencesInNamespace()
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()
{
Expand Down Expand Up @@ -503,6 +510,42 @@ public async Task CanDeleteAReference()
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()
{
Expand Down
68 changes: 67 additions & 1 deletion Octokit.Tests/Clients/ReferencesClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ public async Task RequestsCorrectUrl()
connection.Received().Get<Reference>(Arg.Is<Uri>(u => u.ToString() == "repos/owner/repo/git/refs/heads/develop"));
}

[Fact]
public async Task RequestsCorrectUrlWithRef()
{
var connection = Substitute.For<IApiConnection>();
var client = new ReferencesClient(connection);

await client.Get("owner", "repo", "refs/heads/develop");

connection.Received().Get<Reference>(Arg.Is<Uri>(u => u.ToString() == "repos/owner/repo/git/refs/heads/develop"));
}

[Fact]
public async Task RequestsCorrectUrlWithRepositoryId()
{
Expand All @@ -57,6 +68,17 @@ public async Task RequestsCorrectUrlWithRepositoryId()

connection.Received().Get<Reference>(Arg.Is<Uri>(u => u.ToString() == "repositories/1/git/refs/heads/develop"));
}

[Fact]
public async Task RequestsCorrectUrlWithRepositoryIdAndRefs()
{
var connection = Substitute.For<IApiConnection>();
var client = new ReferencesClient(connection);

await client.Get(1, "refs/heads/develop");

connection.Received().Get<Reference>(Arg.Is<Uri>(u => u.ToString() == "repositories/1/git/refs/heads/develop"));
}
}

public class TheGetAllMethod
Expand Down Expand Up @@ -150,6 +172,17 @@ public async Task RequestsCorrectUrl()
connection.Received().GetAll<Reference>(Arg.Is<Uri>(u => u.ToString() == "repos/owner/repo/git/refs/heads"), Args.ApiOptions);
}

[Fact]
public async Task RequestsCorrectUrlWithRef()
{
var connection = Substitute.For<IApiConnection>();
var client = new ReferencesClient(connection);

await client.GetAllForSubNamespace("owner", "repo", "refs/heads");

connection.Received().GetAll<Reference>(Arg.Is<Uri>(u => u.ToString() == "repos/owner/repo/git/refs/heads"), Args.ApiOptions);
}

[Fact]
public async Task RequestsCorrectUrlWithRepositoryId()
{
Expand All @@ -160,6 +193,17 @@ public async Task RequestsCorrectUrlWithRepositoryId()

connection.Received().GetAll<Reference>(Arg.Is<Uri>(u => u.ToString() == "repositories/1/git/refs/heads"), Args.ApiOptions);
}

[Fact]
public async Task RequestsCorrectUrlWithRepositoryIdAndRefs()
{
var connection = Substitute.For<IApiConnection>();
var client = new ReferencesClient(connection);

await client.GetAllForSubNamespace(1, "refs/heads");

connection.Received().GetAll<Reference>(Arg.Is<Uri>(u => u.ToString() == "repositories/1/git/refs/heads"), Args.ApiOptions);
}
}

public class TheCreateMethod
Expand Down Expand Up @@ -282,6 +326,17 @@ public async Task RequestsCorrectUrl()
connection.Received().Delete(Arg.Is<Uri>(u => u.ToString() == "repos/owner/repo/git/refs/heads/develop"));
}

[Fact]
public async Task RequestsCorrectUrlWithRefs()
{
var connection = Substitute.For<IApiConnection>();
var client = new ReferencesClient(connection);

await client.Delete("owner", "repo", "refs/heads/develop");

connection.Received().Delete(Arg.Is<Uri>(u => u.ToString() == "repos/owner/repo/git/refs/heads/develop"));
}

[Fact]
public async Task RequestsCorrectUrlWithRepositoryId()
{
Expand All @@ -292,6 +347,17 @@ public async Task RequestsCorrectUrlWithRepositoryId()

connection.Received().Delete(Arg.Is<Uri>(u => u.ToString() == "repositories/1/git/refs/heads/develop"));
}

[Fact]
public async Task RequestsCorrectUrlWithRepositoryIdAndRefs()
{
var connection = Substitute.For<IApiConnection>();
var client = new ReferencesClient(connection);

await client.Delete(1, "refs/heads/develop");

connection.Received().Delete(Arg.Is<Uri>(u => u.ToString() == "repositories/1/git/refs/heads/develop"));
}
}
}
}
}
89 changes: 72 additions & 17 deletions Octokit/Clients/IReferencesClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@ public interface IReferencesClient
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="reference">The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1"</param>
/// <returns></returns>
/// <param name="reference">The reference name</param>
/// <remarks>
/// 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")
/// </remarks>
[SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get",
Justification = "Method makes a network request")]
Task<Reference> Get(string owner, string name, string reference);
Expand All @@ -33,8 +38,13 @@ public interface IReferencesClient
/// http://developer.github.com/v3/git/refs/#get-a-reference
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="reference">The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1"</param>
/// <returns></returns>
/// <param name="reference">The reference name</param>
/// <remarks>
/// 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")
/// </remarks>
[SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get",
Justification = "Method makes a network request")]
Task<Reference> Get(long repositoryId, string reference);
Expand Down Expand Up @@ -92,7 +102,12 @@ public interface IReferencesClient
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="subNamespace">The sub-namespace to get references for</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
Task<IReadOnlyList<Reference>> GetAllForSubNamespace(string owner, string name, string subNamespace);

/// <summary>
Expand All @@ -105,7 +120,12 @@ public interface IReferencesClient
/// <param name="name">The name of the repository</param>
/// <param name="subNamespace">The sub-namespace to get references for</param>
/// <param name="options">Options for changing the API response</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
Task<IReadOnlyList<Reference>> GetAllForSubNamespace(string owner, string name, string subNamespace, ApiOptions options);

/// <summary>
Expand All @@ -116,7 +136,12 @@ public interface IReferencesClient
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="subNamespace">The sub-namespace to get references for</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
Task<IReadOnlyList<Reference>> GetAllForSubNamespace(long repositoryId, string subNamespace);

/// <summary>
Expand All @@ -128,7 +153,12 @@ public interface IReferencesClient
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="subNamespace">The sub-namespace to get references for</param>
/// <param name="options">Options for changing the API response</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
Task<IReadOnlyList<Reference>> GetAllForSubNamespace(long repositoryId, string subNamespace, ApiOptions options);

/// <summary>
Expand All @@ -140,7 +170,12 @@ public interface IReferencesClient
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="reference">The reference to create</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
Task<Reference> Create(string owner, string name, NewReference reference);

/// <summary>
Expand All @@ -162,9 +197,14 @@ public interface IReferencesClient
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="reference">The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1"</param>
/// <param name="reference">The reference name</param>
/// <param name="referenceUpdate">The updated reference data</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
Task<Reference> Update(string owner, string name, string reference, ReferenceUpdate referenceUpdate);

/// <summary>
Expand All @@ -174,9 +214,14 @@ public interface IReferencesClient
/// http://developer.github.com/v3/git/refs/#update-a-reference
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="reference">The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1"</param>
/// <param name="reference">The reference name</param>
/// <param name="referenceUpdate">The updated reference data</param>
/// <returns></returns>
/// <remarks>
/// 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")
/// </remarks>
Task<Reference> Update(long repositoryId, string reference, ReferenceUpdate referenceUpdate);

/// <summary>
Expand All @@ -187,8 +232,13 @@ public interface IReferencesClient
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="reference">The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1"</param>
/// <returns></returns>
/// <param name="reference">The reference name</param>
/// <remarks>
/// 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")
/// </remarks>
Task Delete(string owner, string name, string reference);

/// <summary>
Expand All @@ -198,8 +248,13 @@ public interface IReferencesClient
/// http://developer.github.com/v3/git/refs/#delete-a-reference
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="reference">The canonical name of the reference without the 'refs/' prefix. e.g. "heads/master" or "tags/release-1"</param>
/// <returns></returns>
/// <param name="reference">The reference name</param>
/// <remarks>
/// 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")
/// </remarks>
Task Delete(long repositoryId, string reference);
}
}
Loading