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.

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

support redirect requests natively #808

Merged
merged 29 commits into from
Jul 17, 2015
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
14fe8bc
First attempt to reuse HttpClient
darrelmiller May 13, 2015
7f95f55
massaging and fighting with fxcop
shiftkey May 22, 2015
4d77f0e
suppressing a bunch of stuff because murtaugh's law
shiftkey May 22, 2015
f0011cc
missed an overload
shiftkey May 22, 2015
894609e
missed a supresssion
shiftkey May 22, 2015
eb7e14b
test hacks for great good
shiftkey May 22, 2015
ea9f3a4
Fixed issues with RedirectHandler and added tests
darrelmiller May 22, 2015
5d0c8d6
get the portable tests back running by muting all these methods that …
shiftkey May 31, 2015
e5e4b4c
extracting all the cross-platform setup of HttpMessageHandler into
shiftkey May 31, 2015
549ff92
a bit more cleanup
shiftkey May 31, 2015
d551eb2
internalize this for now
shiftkey May 31, 2015
3c32814
:lipstick:
shiftkey May 31, 2015
be57a70
wrote some docs
shiftkey May 31, 2015
d7d7efd
renaming things is hard
shiftkey May 31, 2015
74ac314
review and cleanup suppressions
shiftkey May 31, 2015
3859ff3
the test, it passes
shiftkey May 31, 2015
3acdd10
deprecate the optional AllowAutoRedirect parameter
shiftkey May 31, 2015
54e0865
introduce GetArchive overloads for RepositoryContentClients which han…
shiftkey May 31, 2015
51947ad
added another exclusion for detecting binary content
shiftkey May 31, 2015
9a634e4
docs docs docs
shiftkey May 31, 2015
fe36783
and the other docs
shiftkey May 31, 2015
ea48040
Merge pull request #810 from octokit/fix-redirect-with-archive-link
shiftkey Jun 1, 2015
72339c8
added tests specific to redirects
shiftkey Jun 5, 2015
442bbb4
throw an error if the redirect count has been exceeded
shiftkey Jun 5, 2015
eafd63d
oops, corrected the test
shiftkey Jun 5, 2015
982dca5
specify the timeout interval when downloading the archive
shiftkey Jun 15, 2015
0acdb8c
Merge branch 'master' into redirect-requests
shiftkey Jun 15, 2015
5be1cae
:art: Really anal fix-ups of extra newlines
haacked Jul 17, 2015
8e77f00
:art: Minor formatting changes
haacked Jul 17, 2015
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
33 changes: 33 additions & 0 deletions Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,18 @@ public interface IObservableRepositoryContentsClient
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <returns></returns>
[Obsolete("Use GetArchive to download the archive instead")]
IObservable<string> GetArchiveLink(string owner, string name);

/// <summary>
/// Get an archive of a given repository's contents
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <returns>A promise, containing the binary contents of the archive</returns>
IObservable<byte[]> GetArchive(string owner, string name);

/// <summary>
/// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
/// Please make sure your HTTP framework is configured to follow redirects or you will need to use the
Expand All @@ -47,8 +57,19 @@ public interface IObservableRepositoryContentsClient
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <returns></returns>
[Obsolete("Use GetArchive to download the archive instead")]
IObservable<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat);

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <returns>A promise, containing the binary contents of the archive</returns>
IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat);

/// <summary>
/// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
/// Please make sure your HTTP framework is configured to follow redirects or you will need to use the
Expand All @@ -61,8 +82,20 @@ public interface IObservableRepositoryContentsClient
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <returns></returns>
[Obsolete("Use GetArchive to download the archive instead")]
IObservable<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference);

/// <summary>
/// Get an archive of a given repository's contents, using a specific format and reference
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <returns>A promise, containing the binary contents of the archive</returns>
IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference);

/// <summary>
/// Returns the contents of a file or directory in a repository.
/// </summary>
Expand Down
42 changes: 42 additions & 0 deletions Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,24 @@ public IObservable<string> GetReadmeHtml(string owner, string name)
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <returns></returns>
[Obsolete("Use GetArchive to download the archive instead")]
public IObservable<string> GetArchiveLink(string owner, string name)
{
return GetArchiveLink(owner, name, ArchiveFormat.Tarball, string.Empty);
}

/// <summary>
/// Get an archive of a given repository's contents
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <returns>A promise, containing the binary contents of the archive</returns>
public IObservable<byte[]> GetArchive(string owner, string name)
{
return _client.Repository.Content.GetArchive(owner, name).ToObservable();
}

/// <summary>
/// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
/// Please make sure your HTTP framework is configured to follow redirects or you will need to use the
Expand All @@ -75,11 +88,25 @@ public IObservable<string> GetArchiveLink(string owner, string name)
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <returns></returns>
[Obsolete("Use GetArchive to download the archive instead")]
public IObservable<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat)
{
return GetArchiveLink(owner, name, archiveFormat, String.Empty);
}

/// <summary>
/// Get an archive of a given repository's contents, in a specific format
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <returns>A promise, containing the binary contents of the archive</returns>
public IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat)
{
return _client.Repository.Content.GetArchive(owner, name, archiveFormat).ToObservable();
}

/// <summary>
/// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
/// Please make sure your HTTP framework is configured to follow redirects or you will need to use the
Expand All @@ -92,6 +119,7 @@ public IObservable<string> GetArchiveLink(string owner, string name, ArchiveForm
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <returns></returns>
[Obsolete("Use GetArchive to download the archive instead")]
public IObservable<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference)
{
Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
Expand All @@ -100,6 +128,20 @@ public IObservable<string> GetArchiveLink(string owner, string name, ArchiveForm
return _client.Repository.Content.GetArchiveLink(owner, name, archiveFormat, reference).ToObservable();
}

/// <summary>
/// Get an archive of a given repository's contents, using a specific format and reference
/// </summary>
/// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
/// <param name="reference">A valid Git reference.</param>
/// <returns>A promise, containing the binary contents of the archive</returns>
public IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference)
{
return _client.Repository.Content.GetArchive(owner, name, archiveFormat, reference).ToObservable();
}

/// <summary>
/// Returns the contents of a file or directory in a repository.
/// </summary>
Expand Down
2 changes: 0 additions & 2 deletions Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.Linq;
using System.Threading.Tasks;
using Octokit;
using Octokit.Tests.Helpers;
using Octokit.Tests.Integration;
using Xunit;

Expand Down Expand Up @@ -540,7 +539,6 @@ public async Task ReturnsForkedRepository()
}
}


public class TheGetAllPublicMethod
{
[IntegrationTest(Skip = "Takes too long to run.")]
Expand Down
26 changes: 13 additions & 13 deletions Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,43 +115,43 @@ await Assert.ThrowsAsync<NotFoundException>(
}
}

[IntegrationTest]
public async Task GetsArchiveLinkAsTarball()
[IntegrationTest(Skip = "this will probably take too long")]
public async Task GetsArchiveAsTarball()
{
var github = Helper.GetAuthenticatedClient();

var archiveLink = await github
var archive = await github
.Repository
.Content
.GetArchiveLink("octokit", "octokit.net");
.GetArchive("octokit", "octokit.net");

Assert.Equal("https://codeload.github.com/octokit/octokit.net/legacy.tar.gz/master", archiveLink);
Assert.NotEmpty(archive);
}

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

var archiveLink = await github
var archive = await github
.Repository
.Content
.GetArchiveLink("octokit", "octokit.net", ArchiveFormat.Zipball, "");
.GetArchive("octokit", "octokit.net", ArchiveFormat.Zipball, "");

Assert.Equal("https://codeload.github.com/octokit/octokit.net/legacy.zip/master", archiveLink);
Assert.NotEmpty(archive);
}

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

var archiveLink = await github
var archive = await github
.Repository
.Content
.GetArchiveLink("alfhenrik", "ScriptCs.OctoKit", ArchiveFormat.Tarball, "dev");
.GetArchive("alfhenrik", "ScriptCs.OctoKit", ArchiveFormat.Tarball, "dev");

Assert.Equal("https://codeload.github.com/alfhenrik/ScriptCs.OctoKit/legacy.tar.gz/dev", archiveLink);
Assert.NotEmpty(archive);
}
}
}
4 changes: 2 additions & 2 deletions Octokit.Tests.Integration/HttpClientAdapterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class TheSendAsyncMethod
[IntegrationTest]
public async Task CanDownloadImage()
{
var httpClient = new HttpClientAdapter();
var httpClient = new HttpClientAdapter(HttpMessageHandlerFactory.CreateDefault);
var request = new Request
{
BaseAddress = new Uri("https://github.global.ssl.fastly.net/", UriKind.Absolute),
Expand All @@ -36,7 +36,7 @@ public async Task CanDownloadImage()
[IntegrationTest]
public async Task CanCancelARequest()
{
var httpClient = new HttpClientAdapter();
var httpClient = new HttpClientAdapter(HttpMessageHandlerFactory.CreateDefault);
var request = new Request
{
BaseAddress = new Uri("https://github.global.ssl.fastly.net/", UriKind.Absolute),
Expand Down
1 change: 1 addition & 0 deletions Octokit.Tests.Integration/Octokit.Tests.Integration.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
<Compile Include="Reactive\ObservableRespositoryDeployKeysClientTests.cs" />
<Compile Include="Reactive\ObservableUserEmailsClientTests.cs" />
<Compile Include="Reactive\ObservableTeamsClientTests.cs" />
<Compile Include="RedirectTests.cs" />
<Compile Include="SelfTests.cs" />
</ItemGroup>
<ItemGroup>
Expand Down
45 changes: 45 additions & 0 deletions Octokit.Tests.Integration/RedirectTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
using System.Threading.Tasks;
using Xunit;

namespace Octokit.Tests.Integration
{
public class RedirectTests
{
[IntegrationTest]
public async Task ReturnsRedirectedRepository()
{
var github = Helper.GetAuthenticatedClient();

var repository = await github.Repository.Get("robconery", "massive");

Assert.Equal("https://github.com/FransBouma/Massive.git", repository.CloneUrl);
Assert.False(repository.Private);
Assert.False(repository.Fork);
Assert.Equal(AccountType.User, repository.Owner.Type);
}

[IntegrationTest]
public async Task CanCreateIssueOnRedirectedRepository()
{
var client = Helper.GetAuthenticatedClient();

var owner = "shiftkey-tester";
var oldRepoName = "repository-before-rename";
var newRepoName = "repository-after-rename";

var newIssue = new NewIssue("a test issue") { Body = "A new unassigned issue" };
var issue = await client.Issue.Create(owner, oldRepoName, newIssue);
Assert.NotNull(issue);

Assert.True(issue.Url.AbsoluteUri.Contains("repository-after-rename"));

var resolvedIssue = await client.Issue.Get(owner, newRepoName, issue.Number);

Assert.NotNull(resolvedIssue);

var update = resolvedIssue.ToUpdate();
update.State = ItemState.Closed;
await client.Issue.Update(owner, oldRepoName, issue.Number, update);
}
}
}
2 changes: 1 addition & 1 deletion Octokit.Tests/Http/ConnectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ await connection.Post<string>(
httpClient.Received().Send(Arg.Is<IRequest>(req =>
req.BaseAddress == _exampleUri &&
req.Body == body &&
req.Headers["Accept"] == "application/vnd.github.v3+json; charset=utf-8" &&
req.Headers["Accept"] == "application/vnd.github.quicksilver-preview+json; charset=utf-8, application/vnd.github.v3+json; charset=utf-8" &&
req.ContentType == "application/arbitrary" &&
req.Method == HttpMethod.Post &&
req.Endpoint == new Uri("https://other.host.com/path?query=val")), Args.CancellationToken);
Expand Down
5 changes: 5 additions & 0 deletions Octokit.Tests/Http/HttpClientAdapterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ public async Task SetsContentType()

sealed class HttpClientAdapterTester : HttpClientAdapter
{
public HttpClientAdapterTester()
: base(HttpMessageHandlerFactory.CreateDefault)
{
}

public HttpRequestMessage BuildRequestMessageTester(IRequest request)
{
return BuildRequestMessage(request);
Expand Down
2 changes: 1 addition & 1 deletion Octokit.Tests/Http/JsonHttpPipelineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void SetsRequestAcceptHeader()
jsonPipeline.SerializeRequest(request);

Assert.Contains("Accept", request.Headers.Keys);
Assert.Equal("application/vnd.github.v3+json; charset=utf-8", request.Headers["Accept"]);
Assert.Equal("application/vnd.github.quicksilver-preview+json; charset=utf-8, application/vnd.github.v3+json; charset=utf-8", request.Headers["Accept"]);
}

[Fact]
Expand Down
Loading