-
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 6 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 |
---|---|---|
@@ -0,0 +1,75 @@ | ||
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 IZendeskClient client; | ||
|
||
|
||
private string ResponseType { get; } | ||
|
||
public CursorPaginatedIterator(ICursorPagination<T> response, IZendeskClient 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() | ||
{ | ||
Console.WriteLine("fetching the next page... "); | ||
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. any chance can we remove this Console.WriteLine? Or at least use the dotnet ILogger 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, removed. this was not supposed to be there :) |
||
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.PaginatedResource.GetPage(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,14 @@ | ||
using System.Net.Http; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
namespace ZendeskApi.Client.Resources.Interfaces | ||
{ | ||
public interface IPaginatedResource | ||
{ | ||
Task<HttpResponseMessage> GetPage( | ||
string url, | ||
CancellationToken cancellationToken = default); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
using System.Net.Http; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.Extensions.Logging; | ||
using ZendeskApi.Client.Resources.Interfaces; | ||
|
||
namespace ZendeskApi.Client.Resources | ||
{ | ||
public class PaginatedResource : AbstractBaseResource<PaginatedResource>, IPaginatedResource | ||
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. Just trying to wrap my head around this resource. I can see that this is only used in one place: in the To my mind, these resource classes (and by convention anything implementing In terms of an alternatives I can think off off the bat...
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 🙏 what do you think of my last commit just pushed? I completely removed the the var client = serviceProvider.GetRequiredService<IZendeskClient>();
var apiClient = serviceProvider.GetRequiredService<IZendeskApiClient>();
var ticketCursorResponse = await client.Tickets.GetAllAsync(new CursorPager { Size = 2 });
var iterator = new CursorPaginatedIterator<Ticket>(ticketCursorResponse, apiClient); Hence the changes in the Factory to accommodate the tests. What do you think? 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. @fbvilela This is definitely an improvement. However, I think there is still one issue. The way a consumer typically would use this client is by injecting an instance of
Thoughts? 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, about your second idea, I might be missing something here... would this factory be responsible for creating a another idea, could we perhaps make a new method in the 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. 🙏 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. Let me know if there's anything else we could work on. it would be great to finish off this work :) 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. Hi @fbvilela apologies for the delay here.
Yes! So the issue I have with the current approach is that consumers now need to know about |
||
{ | ||
public PaginatedResource( | ||
IZendeskApiClient apiClient, | ||
ILogger logger) | ||
: base(apiClient, logger, "") | ||
{ } | ||
|
||
public async Task<HttpResponseMessage> GetPage(string url, CancellationToken cancellationToken = default) | ||
{ | ||
using var client = ApiClient.CreateClient(); | ||
return await client.GetAsync(url); | ||
} | ||
} | ||
} | ||
|
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; | ||
} | ||
} |
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 RequestsCursorResponse : CursorPaginationResponse<Request> | ||
{ | ||
[JsonProperty("requests")] | ||
public IEnumerable<Request> Requests { get; set; } | ||
|
||
protected override IEnumerable<Request> Enumerable => Requests; | ||
} | ||
} |
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 TicketAuditCursorResponse : CursorPaginationResponse<TicketAudit> | ||
{ | ||
[JsonProperty("audits")] | ||
public IEnumerable<TicketAudit> Audits { get; set; } | ||
|
||
protected override IEnumerable<TicketAudit> Enumerable => Audits; | ||
} | ||
} |
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?