Support for asynchronous autopagination #1947
Conversation
ob-stripe
force-pushed
the
ob-auto-paging-async
branch
6 times, most recently
from
d00bd3f
to
0bfe8b4
Compare
|
❤️ |
ob-stripe
force-pushed
the
ob-auto-paging-async
branch
from
0bfe8b4
to
143005e
Compare
ob-stripe
changed the title
| string url, | ||
| ListOptions options, | ||
| RequestOptions requestOptions, | ||
| [EnumeratorCancellation] CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
This is not very well documented, but what the EnumeratorCancellation attribute does is add support for two different ways of passing the cancellation token:
- directly as a normal method parameter
- using the
.WithCancellation()extension method
| @@ -38,6 +39,12 @@ | |||
| <CodeAnalysisRuleSet>..\_stylecop\StyleCopRules.ruleset</CodeAnalysisRuleSet> | |||
| </PropertyGroup> | |||
|
|
|||
| <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'"> | |||
| <PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="1.1.0" /> | |||
There was a problem hiding this comment.
This is the backport package that adds support for IAsyncEnumerable in .NET Standard 2.0.
| <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'"> | ||
| <PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="1.1.0" /> | ||
| <PackageReference Include="System.Configuration.ConfigurationManager" Version="4.5.0" /> | ||
| <PackageReference Include="System.Linq.Async" Version="4.0.0" /> |
There was a problem hiding this comment.
This adds support for async enumerators to LINQ methods. We use this to convert the IAsyncEnumerable to a regular IEnumerable (so that the synchronous version ListRequestAutoPaging is just a thin wrapper over the async ListRequestAutoPagingAsync).
|
|
||
| try | ||
| { | ||
| await foreach (var model in service.ListAutoPagingAsync(options).WithCancellation(source.Token)) |
There was a problem hiding this comment.
This illustrates the new await foreach syntax added in C# 8, as well as support for .WithCancellation() to pass cancellation tokens.
| @@ -67,5 +67,12 @@ public virtual IEnumerable<TaxId> ListAutoPaging(string customerId, TaxIdListOpt | |||
| { | |||
| return this.ListNestedEntitiesAutoPaging(customerId, options, requestOptions); | |||
| } | |||
|
|
|||
| #if !NET45 | |||
| public virtual IAsyncEnumerable<TaxId> ListAutoPagingAsync(string customerId, TaxIdListOptions options = null, RequestOptions requestOptions = null, CancellationToken cancellationToken = default) | |||
There was a problem hiding this comment.
Wondering if these customerId parameter names will be easy to derive from the spec, or if it would be easier to use id or parentId in the nested cases...
There was a problem hiding this comment.
They shouldn't need to. In another PR for your codegen branch we renamed all of those to parentId and id instead. It's likely just that ob used a codemod or a regex to generate those
There was a problem hiding this comment.
Yep, this PR is like 90% codemod. Sorry for the additional toil CJ :/
ob-stripe
force-pushed
the
ob-auto-paging-async
branch
from
143005e
to
05ccf68
Compare
ob-stripe
force-pushed
the
ob-auto-paging-async
branch
from
05ccf68
to
b618707
Compare
ob-stripe
force-pushed
the
ob-auto-paging-async
branch
from
b618707
to
493dc83
Compare
stripe-ci
assigned ob-stripe and unassigned remi-stripe, brandur-stripe and cjavilla-stripe
|
Thanks both! |
|
Thank you! |
r? @brandur-stripe @cjavilla-stripe @remi-stripe
cc @stripe/api-libraries
This PR adds support for asynchronous autopagination, using the new
IAsyncEnumerable<T>interface introduced in C# 8.Normally, this would require .NET Standard 2.1, but we can use it without changing our target of .NET Standard 2.0 thanks to the Microsoft.Bcl.AsyncInterfaces package. Unfortunately there is (AFAIK) no way to support this with .NET Framework 4.5, so I had to add a gazillion
#if !NET45preprocessor macros.This PR is very large and I should have done a better job of splitting it into smaller commits, but it shouldn't be too hard to review:
Service.cs, in the newListRequestAutoPagingAsync<T>method: https://github.com/stripe/stripe-dotnet/pull/1947/files#diff-0de0406b0f5569892ed33a084f9759ceR275. The implementation is very similar to the old synchronousListRequestAutoPaging<T>method. The main differences are:RequestAsyncinstead ofRequestto fetch the pagesCancellationTokenparameter, and there are a couple ofcancellationToken.ThrowIfCancellationRequested();in the loops to check if we need to stop iteratingService.cshas a newListEntitiesAutoPagingAsynchelper method: https://github.com/stripe/stripe-dotnet/pull/1947/files#diff-0de0406b0f5569892ed33a084f9759ceR169, and similarlyServiceNested.cshas a newListNestedEntitiesAutoPagingAsynchelper method: https://github.com/stripe/stripe-dotnet/pull/1947/files#diff-efcfb49f6a90c39650225af44e8ad09fR143IListableandINestedListableinterfaces were updated with a newListAutoPagingAsyncsignatureListAutoPagingAsyncmethodsFor tests:
AutoPagingTest.cswas updated to add tests for the new async methods.ListAutoPagingAsync,ListAutoPagingAsync_NoParams,ListAutoPagingAsync_StartingAfterandListAutoPagingAsync_EndingBeforeare copies of the tests for the synchronous methods.ListAutoPagingAsync_WithCancellationis a new test unique to the async version to test the cancellation behavior.ListAutoPagingtest in service test classes was updated to useFirst()instead ofToList().ToList()forces the conversion of theIEnumerableinto aList, which could cause many requests if there were many pages to fetch. This is not actually an issue when using stripe-mock, but better safe than sorryListAutoPagingAsynctests were added to all service test classes.Fixes #1946.