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

[Next Major] prevent autopagination from mutating #2899

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions src/Stripe.net/Services/_base/BaseOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public class BaseOptions : INestedOptions
public IDictionary<string, object> ExtraParams { get; set; }
= new Dictionary<string, object>();

internal BaseOptions Clone()
{
return (BaseOptions)this.MemberwiseClone();
}

/// <summary>
/// Adds an <c>expand</c> value to the request, to request expansion of a specific
/// field in the response. When requesting expansions in a list request, don't forget
Expand Down
8 changes: 4 additions & 4 deletions src/Stripe.net/Services/_base/Service.cs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ protected TEntityReturned CreateEntity(BaseOptions options, RequestOptions reque
options,
requestOptions);

options = options ?? new ListOptions();
options = ((ListOptions)options?.Clone()) ?? new ListOptions();
bool iterateBackward = false;

// Backward iterating activates if we have an `EndingBefore`
Expand Down Expand Up @@ -397,7 +397,7 @@ protected TEntityReturned CreateEntity(BaseOptions options, RequestOptions reque
requestOptions,
cancellationToken);

options = options ?? new ListOptions();
options = ((ListOptions)options?.Clone()) ?? new ListOptions();
bool iterateBackward = false;

// Backward iterating activates if we have an `EndingBefore`
Expand Down Expand Up @@ -485,7 +485,7 @@ protected TEntityReturned CreateEntity(BaseOptions options, RequestOptions reque
options,
requestOptions);

options = options ?? new SearchOptions();
options = ((SearchOptions)options?.Clone()) ?? new SearchOptions();

while (true)
{
Expand Down Expand Up @@ -533,7 +533,7 @@ protected TEntityReturned CreateEntity(BaseOptions options, RequestOptions reque
requestOptions,
cancellationToken);

options = options ?? new SearchOptions();
options = ((SearchOptions)options?.Clone()) ?? new SearchOptions();

while (true)
{
Expand Down
6 changes: 6 additions & 0 deletions src/StripeTests/Services/AutoPagingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ public void ListAutoPaging_StartingAfter()
Assert.Equal("pm_126", models[1].Id);
Assert.Equal("pm_127", models[2].Id);

// Should not mutate its argument
Assert.Equal("pm_124", options.StartingAfter);

// Check invocations
this.MockHttpClientFixture.MockHandler.Protected()
.Verify(
Expand Down Expand Up @@ -247,6 +250,9 @@ public void ListAutoPaging_EndingBefore()
};
var models = service.ListAutoPaging(options).ToList();

// Should not mutate its argument
Assert.Equal("pm_127", options.EndingBefore);

// Check results
Assert.Equal(5, models.Count);
Assert.Equal("pm_126", models[0].Id);
Expand Down
10 changes: 8 additions & 2 deletions src/StripeTests/Services/_base/ServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,17 @@ public void SearchAuto_ReturnsAllPages()
"?page=page2&query=my+query");

var service = new TestService(this.StripeClient);
var options = new SearchOptions() { Query = "my query" };

HashSet<string> ids = new HashSet<string>();
foreach (var testEntity in service.SearchAutoPaging(new SearchOptions() { Query = "my query" }))
foreach (var testEntity in service.SearchAutoPaging(options))
{
Assert.NotNull(testEntity);
ids.Add(testEntity.Id);
}

Assert.Null(options.Page);

Assert.Equal(4, ids.Count);
Assert.Equal(4, ids.Count);
}
Expand All @@ -118,13 +121,16 @@ public async Task SearchAutoAsync_ReturnsAllPages()

var service = new TestService(this.StripeClient);

var options = new SearchOptions() { Query = "my query" };
HashSet<string> ids = new HashSet<string>();
await foreach (var testEntity in service.SearchAutoPagingAsync(new SearchOptions() { Query = "my query" }))
await foreach (var testEntity in service.SearchAutoPagingAsync(options))
{
Assert.NotNull(testEntity);
ids.Add(testEntity.Id);
}

Assert.Null(options.Page);

Assert.Equal(4, ids.Count);
}

Expand Down