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

Add cancellation token support for release assets uploading #2267

Merged
merged 4 commits into from Feb 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion Octokit.Reactive/Clients/IObservableReleasesClient.cs
@@ -1,6 +1,7 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.Reactive;
using System.Threading;

namespace Octokit.Reactive
{
Expand Down Expand Up @@ -250,8 +251,9 @@ public interface IObservableReleasesClient
/// </remarks>
/// <param name="release">The <see cref="Release"/> to attach the uploaded asset to</param>
/// <param name="data">Description of the asset with its data</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
IObservable<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data);
IObservable<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data, CancellationToken cancellationToken = default);

/// <summary>
/// Gets the specified <see cref="ReleaseAsset"/> for the specified release of the specified repository.
Expand Down
6 changes: 4 additions & 2 deletions Octokit.Reactive/Clients/ObservableReleasesClient.cs
@@ -1,6 +1,7 @@
using System;
using System.Reactive;
using System.Reactive.Threading.Tasks;
using System.Threading;
using Octokit.Reactive.Internal;

namespace Octokit.Reactive
Expand Down Expand Up @@ -400,13 +401,14 @@ public IObservable<ReleaseAsset> GetAsset(long repositoryId, int assetId)
/// </remarks>
/// <param name="release">The <see cref="Release"/> to attach the uploaded asset to</param>
/// <param name="data">Description of the asset with its data</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
public IObservable<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data)
public IObservable<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data, CancellationToken cancellationToken = default)
{
Ensure.ArgumentNotNull(release, nameof(release));
Ensure.ArgumentNotNull(data, nameof(data));

return _client.UploadAsset(release, data).ToObservable();
return _client.UploadAsset(release, data, cancellationToken).ToObservable();
}

/// <summary>
Expand Down
38 changes: 38 additions & 0 deletions Octokit.Tests/Clients/ReleasesClientTests.cs
@@ -1,7 +1,9 @@
using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using NSubstitute;
using Octokit.Internal;
using Xunit;

namespace Octokit.Tests.Clients
Expand Down Expand Up @@ -484,6 +486,42 @@ public async Task OverrideDefaultTimeout()

apiConnection.Received().Post<ReleaseAsset>(Arg.Any<Uri>(), uploadData.RawData, Arg.Any<string>(), uploadData.ContentType, newTimeout);
}

[Fact]
public async Task CanBeCancelled()
{
var httpClient = new CancellationTestHttpClient();
var connection = new Connection(new ProductHeaderValue("TEST"), httpClient);
var apiConnection = new ApiConnection(connection);

var fixture = new ReleasesClient(apiConnection);

var release = new Release("https://uploads.github.com/anything");
var uploadData = new ReleaseAssetUpload("good", "good/good", Stream.Null, null);

using (var cts = new CancellationTokenSource())
{
var uploadTask = fixture.UploadAsset(release, uploadData, cts.Token);

cts.Cancel();

await Assert.ThrowsAsync<TaskCanceledException>(() => uploadTask);
}
}

private class CancellationTestHttpClient : IHttpClient
{
public async Task<IResponse> Send(IRequest request, CancellationToken cancellationToken)
{
await Task.Delay(TimeSpan.FromSeconds(1), cancellationToken);

throw new Exception("HTTP operation was not cancelled");
}

public void Dispose() { }

public void SetRequestTimeout(TimeSpan timeout) { }
}
Comment on lines +512 to +524
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't figure out how to set this up using NSubstitute. Hence, this mock of IHttpClient.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine 👍🏻

Choose a reason for hiding this comment

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

fuckd up

  • [ ]

}

public class TheGetAssetMethod
Expand Down
4 changes: 3 additions & 1 deletion Octokit/Clients/IReleasesClient.cs
@@ -1,5 +1,6 @@
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using System.Threading.Tasks;

namespace Octokit
Expand Down Expand Up @@ -250,8 +251,9 @@ public interface IReleasesClient
/// </remarks>
/// <param name="release">The <see cref="Release"/> to attach the uploaded asset to</param>
/// <param name="data">Description of the asset with its data</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
Task<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data);
Task<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data, CancellationToken cancellationToken = default);

/// <summary>
/// Gets the specified <see cref="ReleaseAsset"/> for the specified release of the specified repository.
Expand Down
10 changes: 7 additions & 3 deletions Octokit/Clients/ReleasesClient.cs
@@ -1,5 +1,6 @@
using System.Threading.Tasks;
using System.Collections.Generic;
using System.Threading;

namespace Octokit
{
Expand Down Expand Up @@ -398,9 +399,10 @@ public Task<IReadOnlyList<ReleaseAsset>> GetAllAssets(long repositoryId, int id,
/// </remarks>
/// <param name="release">The <see cref="Release"/> to attach the uploaded asset to</param>
/// <param name="data">Description of the asset with its data</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
[ManualRoute("POST", "{server}/repos/{owner}/{repo}/releases/{release_id}/assets")]
public Task<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data)
public Task<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data, CancellationToken cancellationToken = default)
{
Ensure.ArgumentNotNull(release, nameof(release));
Ensure.ArgumentNotNull(data, nameof(data));
Expand All @@ -414,14 +416,16 @@ public Task<ReleaseAsset> UploadAsset(Release release, ReleaseAssetUpload data)
data.RawData,
AcceptHeaders.StableVersion,
data.ContentType,
data.Timeout.GetValueOrDefault());
data.Timeout.GetValueOrDefault(),
cancellationToken);
}

return ApiConnection.Post<ReleaseAsset>(
endpoint,
data.RawData,
AcceptHeaders.StableVersion,
data.ContentType);
data.ContentType,
cancellationToken);
}

/// <summary>
Expand Down
34 changes: 20 additions & 14 deletions Octokit/Http/ApiConnection.cs
Expand Up @@ -216,27 +216,29 @@ public Task<IReadOnlyList<T>> GetAll<T>(Uri uri, IDictionary<string, string> par
/// Creates a new API resource in the list at the specified URI.
/// </summary>
/// <param name="uri">URI endpoint to send request to</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <returns><seealso cref="HttpStatusCode"/>Representing the received HTTP response</returns>
/// <exception cref="ApiException">Thrown when an API error occurs.</exception>
public Task Post(Uri uri)
public Task Post(Uri uri, CancellationToken cancellationToken = default)
{
Ensure.ArgumentNotNull(uri, nameof(uri));

return Connection.Post(uri);
return Connection.Post(uri, cancellationToken);
}

/// <summary>
/// Creates a new API resource in the list at the specified URI.
/// </summary>
/// <typeparam name="T">The API resource's type.</typeparam>
/// <param name="uri">URI of the API resource to get</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <returns>The created API resource.</returns>
/// <exception cref="ApiException">Thrown when an API error occurs.</exception>
public async Task<T> Post<T>(Uri uri)
public async Task<T> Post<T>(Uri uri, CancellationToken cancellationToken = default)
{
Ensure.ArgumentNotNull(uri, nameof(uri));

var response = await Connection.Post<T>(uri).ConfigureAwait(false);
var response = await Connection.Post<T>(uri, cancellationToken).ConfigureAwait(false);
return response.Body;
}

Expand All @@ -246,14 +248,15 @@ public async Task<T> Post<T>(Uri uri)
/// <typeparam name="T">The API resource's type.</typeparam>
/// <param name="uri">URI of the API resource to get</param>
/// <param name="data">Object that describes the new API resource; this will be serialized and used as the request's body</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <returns>The created API resource.</returns>
/// <exception cref="ApiException">Thrown when an API error occurs.</exception>
public Task<T> Post<T>(Uri uri, object data)
public Task<T> Post<T>(Uri uri, object data, CancellationToken cancellationToken = default)
{
Ensure.ArgumentNotNull(uri, nameof(uri));
Ensure.ArgumentNotNull(data, nameof(data));

return Post<T>(uri, data, null, null);
return Post<T>(uri, data, null, null, cancellationToken);
}

/// <summary>
Expand All @@ -263,11 +266,12 @@ public Task<T> Post<T>(Uri uri, object data)
/// <param name="uri">URI of the API resource to get</param>
/// <param name="data">Object that describes the new API resource; this will be serialized and used as the request's body</param>
/// <param name="accepts">Accept header to use for the API request</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <returns>The created API resource.</returns>
/// <exception cref="ApiException">Thrown when an API error occurs.</exception>
public Task<T> Post<T>(Uri uri, object data, string accepts)
public Task<T> Post<T>(Uri uri, object data, string accepts, CancellationToken cancellationToken = default)
{
return Post<T>(uri, data, accepts, null);
return Post<T>(uri, data, accepts, null, cancellationToken);
}

/// <summary>
Expand All @@ -278,14 +282,15 @@ public Task<T> Post<T>(Uri uri, object data, string accepts)
/// <param name="data">Object that describes the new API resource; this will be serialized and used as the request's body</param>
/// <param name="accepts">Accept header to use for the API request</param>
/// <param name="contentType">Content type of the API request</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <returns>The created API resource.</returns>
/// <exception cref="ApiException">Thrown when an API error occurs.</exception>
public async Task<T> Post<T>(Uri uri, object data, string accepts, string contentType)
public async Task<T> Post<T>(Uri uri, object data, string accepts, string contentType, CancellationToken cancellationToken = default)
{
Ensure.ArgumentNotNull(uri, nameof(uri));
Ensure.ArgumentNotNull(data, nameof(data));

var response = await Connection.Post<T>(uri, data, accepts, contentType).ConfigureAwait(false);
var response = await Connection.Post<T>(uri, data, accepts, contentType, cancellationToken: cancellationToken).ConfigureAwait(false);
return response.Body;
}

Expand All @@ -298,25 +303,26 @@ public async Task<T> Post<T>(Uri uri, object data, string accepts, string conten
/// <param name="accepts">Accept header to use for the API request</param>
/// <param name="contentType">Content type of the API request</param>
/// <param name="twoFactorAuthenticationCode">Two Factor Authentication Code</param>
/// <param name="cancellationToken">An optional token to monitor for cancellation requests</param>
/// <returns>The created API resource.</returns>
/// <exception cref="ApiException">Thrown when an API error occurs.</exception>
public async Task<T> Post<T>(Uri uri, object data, string accepts, string contentType, string twoFactorAuthenticationCode)
public async Task<T> Post<T>(Uri uri, object data, string accepts, string contentType, string twoFactorAuthenticationCode, CancellationToken cancellationToken = default)
{
Ensure.ArgumentNotNull(uri, nameof(uri));
Ensure.ArgumentNotNull(data, nameof(data));
Ensure.ArgumentNotNull(twoFactorAuthenticationCode, nameof(twoFactorAuthenticationCode));

var response = await Connection.Post<T>(uri, data, accepts, contentType, twoFactorAuthenticationCode).ConfigureAwait(false);
var response = await Connection.Post<T>(uri, data, accepts, contentType, twoFactorAuthenticationCode, cancellationToken).ConfigureAwait(false);
return response.Body;
}


public async Task<T> Post<T>(Uri uri, object data, string accepts, string contentType, TimeSpan timeout)
public async Task<T> Post<T>(Uri uri, object data, string accepts, string contentType, TimeSpan timeout, CancellationToken cancellationToken = default)
{
Ensure.ArgumentNotNull(uri, nameof(uri));
Ensure.ArgumentNotNull(data, nameof(data));

var response = await Connection.Post<T>(uri, data, accepts, contentType, timeout).ConfigureAwait(false);
var response = await Connection.Post<T>(uri, data, accepts, contentType, timeout, cancellationToken).ConfigureAwait(false);
return response.Body;
}

Expand Down