-
Notifications
You must be signed in to change notification settings - Fork 86
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
[RED-1966] Adds support to an iterator and more CBP endpoints #322
Changes from 10 commits
ac7f6df
f28acfe
f3484a6
1ef97bd
1914506
ad30118
69f86c3
ee80494
8d03f2e
5d26c02
ab87126
0af94f4
12dab12
20dd4ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,4 +29,4 @@ public interface IZendeskClient | |
ILocaleResource Locales { get; } | ||
ITagsResource Tags { get; } | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
using System; | ||
using System.Collections; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Threading.Tasks; | ||
using Newtonsoft.Json; | ||
using ZendeskApi.Client; | ||
using ZendeskApi.Client.Responses; | ||
|
||
public class CursorPaginatedIterator<T> : IEnumerable<T> | ||
{ | ||
|
||
public ICursorPagination<T> Response { get; set; } | ||
|
||
private IZendeskApiClient client; | ||
|
||
|
||
private string ResponseType { get; } | ||
|
||
public CursorPaginatedIterator(ICursorPagination<T> response, IZendeskApiClient client) | ||
{ | ||
Response = response; | ||
this.client = client; | ||
ResponseType = response.GetType().FullName; | ||
} | ||
|
||
public bool HasMore() => Response.Meta.HasMore; | ||
|
||
public IEnumerator<T> GetEnumerator() | ||
{ | ||
return Response.GetEnumerator(); | ||
} | ||
|
||
IEnumerator IEnumerable.GetEnumerator() | ||
{ | ||
return Response.GetEnumerator(); | ||
} | ||
|
||
|
||
public async Task NextPage() | ||
{ | ||
await ExecuteRequest(Response.Links.Next); | ||
} | ||
|
||
public async Task PrevPage() | ||
{ | ||
await ExecuteRequest(Response.Links.Prev); | ||
} | ||
|
||
public async IAsyncEnumerable<T> All() | ||
{ | ||
foreach (var item in Response) | ||
{ | ||
yield return item; | ||
} | ||
while (HasMore()) | ||
{ | ||
await NextPage(); | ||
foreach (var item in Response) | ||
{ | ||
yield return item; | ||
} | ||
} | ||
yield break; | ||
} | ||
|
||
private async Task ExecuteRequest(string requestUrl) | ||
{ | ||
var httpResponseMessage = await client.CreateClient().GetAsync(requestUrl); | ||
var responseBody = await httpResponseMessage.Content.ReadAsStringAsync(); | ||
Response = (ICursorPagination<T>)JsonConvert.DeserializeObject(responseBody, Type.GetType(ResponseType)); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
using System; | ||
using Microsoft.Extensions.DependencyInjection; | ||
using ZendeskApi.Client.Responses; | ||
|
||
namespace ZendeskApi.Client.Models | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can probably move the iterator/factory to a more suitable folder/namespace. Perhaps something like |
||
{ | ||
public interface ICursorPaginatedIteratorFactory | ||
{ | ||
CursorPaginatedIterator<T> Create<T>(ICursorPagination<T> response); | ||
} | ||
|
||
public class CursorPaginatedIteratorFactory : ICursorPaginatedIteratorFactory | ||
{ | ||
private static IServiceProvider serviceProvider; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we are getting closer to the end result here. There are still a few points I would like to make... This namespace ZendeskApi.Client.Pagination
{
public interface ICursorPaginatedIteratorFactory
{
CursorPaginatedIterator<T> Create<T>(ICursorPagination<T> response);
}
public class CursorPaginatedIteratorFactory : ICursorPaginatedIteratorFactory
{
// readonly field as only set in constructor
private readonly IZendeskApiClient zendeskApiClient;
public CursorPaginatedIteratorFactory(IZendeskApiClient _zendeskApiClient)
{
// inject IZendeskApiClient after registering in the container
zendeskApiClient = _zendeskApiClient;
}
public CursorPaginatedIterator<T> Create<T>(ICursorPagination<T> response)
{
return new CursorPaginatedIterator<T>(response, zendeskApiClient);
}
}
} This means a consumer looks like the following: public class YourConsumerClass
{
private readonly CursorPaginatedIteratorFactory iteratorFactory;
public YourConsumerClass(ICursorPaginatedIteratorFactory _iteratorFactory)
{
iteratorFactory = _iteratorFactory;
}
public void YourMethod(ICursorPagination<YourType> response)
{
// Use the factory to get the iterator
CursorPaginatedIterator<YourType> iterator = iteratorFactory.Create(response);
// Rest of your logic using the iterator...
}
} To summarise, from where you are we may consider
AsideI want to reiterate that, in an ideal world, consumers would be able to iterate over the response from paginated endpoints without injecting another dependency on top of Another approach that works around this is to use the adapter pattern. At the moment the CBP endpoints return client.AdaptedTicketAudit.GetAllAsync() where the Not suggesting this adapter as an approach because the amount of code changes required here is massive and I prefer the formerly suggested. But it just came to my mind so wanted to share in case it sparked anything. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks @mikerogers123, I believe I made the (small) changes you requested 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for that @fbvilela the changes look good overall. Have added a few more minor comments to sort, then we should be close to closing this one |
||
|
||
public CursorPaginatedIteratorFactory(IServiceProvider _serviceProvider) | ||
{ | ||
serviceProvider = _serviceProvider; | ||
} | ||
|
||
public CursorPaginatedIterator<T> Create<T>(ICursorPagination<T> response) | ||
{ | ||
return new CursorPaginatedIterator<T>(response, serviceProvider.GetRequiredService<IZendeskApiClient>()); | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
using System; | ||
using System.Collections; | ||
using System.Collections.Generic; | ||
using Newtonsoft.Json; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
using System.Collections.Generic; | ||
using Newtonsoft.Json; | ||
using ZendeskApi.Client.Models; | ||
|
||
namespace ZendeskApi.Client.Responses | ||
{ | ||
[JsonObject] | ||
public class JobStatusListCursorResponse : CursorPaginationResponse<JobStatusResponse> | ||
{ | ||
[JsonProperty("job_statuses")] | ||
public IEnumerable<JobStatusResponse> JobStatuses { get; set; } | ||
|
||
protected override IEnumerable<JobStatusResponse> Enumerable => JobStatuses; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a namespace to this class? And then move it into the Pagination folder?