(fix) RepositoryContentsClient.GetArchive does not return the expected binary content (#2803)

* (GH-2802) Add unit test to demonstrate the problem

This unit test currently fails which demonstrates the problem and it should pass when I am done with the PR

* (GH-2802) Add a GetRaw method that accepts a timeout

I debated adding an optional parameter for the timeout to the existing GetRaw method (which would be my personal preference) but it would be a breaking change and I doubt the Octokit team would be interested in such a change

* (GH-2802) Invoke `GetRaw` rather than `Get>byte[]>` when retrieving a repo's archive content.

This ensure stream content are handled properly and solves the problem described in GitHub issue 2802

* (GH-2802) Fix unit tests that got broken due to my recent change to the GetArchive method

* (GH-2802) Fix formatting

* (GH-2802) Fix more formatting
This commit is contained in:
Jericho
2023-10-16 13:38:12 -04:00
committed by GitHub
parent 7b3abda711
commit 1eac8315ff
4 changed files with 59 additions and 12 deletions
@@ -4,6 +4,8 @@ using System.Threading.Tasks;
using NSubstitute;
using Xunit;
using System.Collections.Generic;
using System.Net;
using Octokit.Internal;
namespace Octokit.Tests.Clients
{
@@ -748,7 +750,7 @@ namespace Octokit.Tests.Clients
const string expectedUri = "repos/org/repo/tarball/";
var expectedTimeSpan = TimeSpan.FromMinutes(60);
connection.Connection.Received().Get<byte[]>(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
connection.Connection.Received().GetRaw(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), null, Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
}
[Fact]
@@ -762,7 +764,7 @@ namespace Octokit.Tests.Clients
const string expectedUri = "repositories/1/tarball/";
var expectedTimeSpan = TimeSpan.FromMinutes(60);
connection.Connection.Received().Get<byte[]>(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
connection.Connection.Received().GetRaw(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), null, Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
}
[Fact]
@@ -776,7 +778,7 @@ namespace Octokit.Tests.Clients
const string expectedUri = "repos/org/repo/zipball/";
var expectedTimeSpan = TimeSpan.FromMinutes(60);
connection.Connection.Received().Get<byte[]>(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
connection.Connection.Received().GetRaw(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), null, Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
}
[Fact]
@@ -790,7 +792,7 @@ namespace Octokit.Tests.Clients
const string expectedUri = "repositories/1/zipball/";
var expectedTimeSpan = TimeSpan.FromMinutes(60);
connection.Connection.Received().Get<byte[]>(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
connection.Connection.Received().GetRaw(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), null, Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
}
[Fact]
@@ -804,7 +806,7 @@ namespace Octokit.Tests.Clients
const string expectedUri = "repos/org/repo/zipball/ref";
var expectedTimeSpan = TimeSpan.FromMinutes(60);
connection.Connection.Received().Get<byte[]>(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
connection.Connection.Received().GetRaw(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), null, Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
}
[Fact]
@@ -818,7 +820,7 @@ namespace Octokit.Tests.Clients
const string expectedUri = "repositories/1/zipball/ref";
var expectedTimeSpan = TimeSpan.FromMinutes(60);
connection.Connection.Received().Get<byte[]>(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
connection.Connection.Received().GetRaw(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), null, Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
}
[Fact]
@@ -832,7 +834,7 @@ namespace Octokit.Tests.Clients
const string expectedUri = "repos/org/repo/zipball/ref";
var expectedTimeSpan = TimeSpan.FromMinutes(60);
connection.Connection.Received().Get<byte[]>(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
connection.Connection.Received().GetRaw(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), null, Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
}
[Fact]
@@ -846,7 +848,7 @@ namespace Octokit.Tests.Clients
const string expectedUri = "repositories/1/zipball/ref";
var expectedTimeSpan = TimeSpan.FromMinutes(60);
connection.Connection.Received().Get<byte[]>(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
connection.Connection.Received().GetRaw(Arg.Is<Uri>(uri => uri.ToString() == expectedUri), null, Arg.Is<TimeSpan>(span => span == expectedTimeSpan));
}
[Fact]
@@ -882,6 +884,27 @@ namespace Octokit.Tests.Clients
await Assert.ThrowsAsync<ArgumentException>(() => client.GetArchive(1, ArchiveFormat.Tarball, "ref", TimeSpan.Zero));
}
[Fact]
public async Task ReturnsExpectedContent()
{
var headers = new Dictionary<string, string>();
var response = TestSetup.CreateResponse(HttpStatusCode.OK, new byte[] { 1, 2, 3, 4 }, headers);
var responseTask = Task.FromResult<IApiResponse<byte[]>>(new ApiResponse<byte[]>(response));
var connection = Substitute.For<IConnection>();
connection.GetRaw(Arg.Is<Uri>(u => u.ToString() == "repos/org/repo/tarball/"), null, TimeSpan.FromMinutes(60))
.Returns(responseTask);
var apiConnection = Substitute.For<IApiConnection>();
apiConnection.Connection.Returns(connection);
var client = new RepositoryContentsClient(apiConnection);
var actual = await client.GetArchive("org", "repo");
Assert.Equal(new byte[] { 1, 2, 3, 4 }, actual);
}
}
}
}
+2 -2
View File
@@ -395,7 +395,7 @@ namespace Octokit
var endpoint = ApiUrls.RepositoryArchiveLink(owner, name, archiveFormat, reference);
var response = await Connection.Get<byte[]>(endpoint, timeout).ConfigureAwait(false);
var response = await Connection.GetRaw(endpoint, null, timeout).ConfigureAwait(false);
return response.Body;
}
@@ -416,7 +416,7 @@ namespace Octokit
var endpoint = ApiUrls.RepositoryArchiveLink(repositoryId, archiveFormat, reference);
var response = await Connection.Get<byte[]>(endpoint, timeout).ConfigureAwait(false);
var response = await Connection.GetRaw(endpoint, null, timeout).ConfigureAwait(false);
return response.Body;
}
+15 -1
View File
@@ -242,7 +242,21 @@ namespace Octokit
Endpoint = uri.ApplyParameters(parameters)
});
}
/// <inheritdoc/>
public Task<IApiResponse<byte[]>> GetRaw(Uri uri, IDictionary<string, string> parameters, TimeSpan timeout)
{
Ensure.ArgumentNotNull(uri, nameof(uri));
return GetRaw(new Request
{
Method = HttpMethod.Get,
BaseAddress = BaseAddress,
Endpoint = uri.ApplyParameters(parameters),
Timeout = timeout
});
}
/// <inheritdoc/>
public Task<IApiResponse<Stream>> GetRawStream(Uri uri, IDictionary<string, string> parameters)
{
+11 -1
View File
@@ -30,7 +30,17 @@ namespace Octokit
/// <returns><seealso cref="IResponse"/> representing the received HTTP response</returns>
/// <remarks>The <see cref="IResponse.Body"/> property will be <c>null</c> if the <paramref name="uri"/> points to a directory instead of a file</remarks>
Task<IApiResponse<byte[]>> GetRaw(Uri uri, IDictionary<string, string> parameters);
/// <summary>
/// Performs an asynchronous HTTP GET request that expects a <seealso cref="IResponse"/> containing raw data.
/// </summary>
/// <param name="uri">URI endpoint to send request to</param>
/// <param name="parameters">Querystring parameters for the request</param>
/// <param name="timeout">The Timeout value</param>
/// <returns><seealso cref="IResponse"/> representing the received HTTP response</returns>
/// <remarks>The <see cref="IResponse.Body"/> property will be <c>null</c> if the <paramref name="uri"/> points to a directory instead of a file</remarks>
Task<IApiResponse<byte[]>> GetRaw(Uri uri, IDictionary<string, string> parameters, TimeSpan timeout);
/// <summary>
/// Performs an asynchronous HTTP GET request that expects a <seealso cref="IResponse"/> containing raw data.
/// </summary>