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

[RED-1966] Adds support to an iterator and more CBP endpoints #322

Merged
merged 14 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 48 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ A .netstandard NuGet package for use with the Zendesk v2 API.

#### The deprecation and replacement of Status API endpoints

More detailed information on the exact changes and motivation can be found [here](https://support.zendesk.com/hc/en-us/articles/5414949730842).
More detailed information on the exact changes and motivation can be found [here](https://support.zendesk.com/hc/en-us/articles/5414949730842).

For the sake of this library, it means the following methods have been removed:

Expand Down Expand Up @@ -50,6 +50,7 @@ Groups:
Help Center:
- GET api/v2/help_center/articles
- GET api/v2/help_center/categories
- GET api/v2/help_center/sections

Organization:
- GET /api/v2/organization_fields
Expand All @@ -63,11 +64,19 @@ Tickets:
- GET /api/v2/tickets
- GET /api/v2/tickets/{ticketId}/comments
- GET /api/v2/ticket_fields
- GET /api/v2/ticket_audits - [Cursor Variant](https://developer.zendesk.com/api-reference/ticketing/tickets/ticket_audits/#pagination)
- GET /api/v2/ticket_audits
- GET

Satisfaction ratings:
- GET /api/v2/satisfaction_ratings

Requests
- GET /api/v2/requests
- GET /api/v2/requests/{requestId}/comments

Job Statuses
- GET /api/v2/job_statuses

[Further reading on Zendesk Pagination changes](https://support.zendesk.com/hc/en-us/articles/4402610093338-Introducing-Pagination-Changes-Zendesk-API)

## 3.x.x
Expand Down Expand Up @@ -126,6 +135,43 @@ await client.Search.SearchAsync<Ticket>(q =>
);
```

## Using Cursor Based Pagination
You can use the `CursorPaginatedIterator` to loop through multiple pages as shown below:
```c#

var services = new ServiceCollection();
services.AddZendeskClientWithHttpClientFactory("https://yoursubomain.zendesk.com", "your@email.com", "your_token_");
var serviceProvider = services.BuildServiceProvider();
var client = serviceProvider.GetRequiredService<IZendeskClient>();

var ticketCursorResponse = await client.Tickets.GetAllAsync(new CursorPager { Size = 5 }); // low page number to force pagination

var iteratorFactory = serviceProvider.GetRequiredService<ICursorPaginatedIteratorFactory>();
// creates the iterator with the response object of the first request
var iterator = iteratorFactory.Create<Ticket>(ticketCursorResponse);

foreach (var ticket in iterator)
{
Console.WriteLine("the id of this ticket is:" + ticket.Id);
} // this loop will stop at the first page

while (iterator.HasMore()) // to loop through all pages
{
await iterator.NextPage();
foreach (var ticket in iterator)
{
Console.WriteLine("the id of this ticket is:" + ticket.Id);
}
}

// alternatively you can use .All() from the iterator
await foreach (var ticket in iterator.All())
{
Console.WriteLine("the id of this ticket is:" + ticket.Id);
}

```

## The Zendesk API

The zendesk api documentation is available at http://developer.zendesk.com/documentation/rest_api/introduction.html
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Net.Http;
using Microsoft.Extensions.DependencyInjection;
using ZendeskApi.Client.Models;
using ZendeskApi.Client.Options;
#pragma warning disable 618

Expand Down Expand Up @@ -33,6 +34,7 @@ public static IServiceCollection AddZendeskClientWithHttpClientFactory(this ISer
{
services.AddScoped<IZendeskClient, ZendeskClient>();
services.AddScoped<IZendeskApiClient, ZendeskApiClientFactory>();
services.AddScoped<ICursorPaginatedIteratorFactory, CursorPaginatedIteratorFactory>();

services.AddHttpClient("zendeskApiClient", c =>
{
Expand Down
2 changes: 1 addition & 1 deletion src/ZendeskApi.Client/IZendeskClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ public interface IZendeskClient
ILocaleResource Locales { get; }
ITagsResource Tags { get; }
}
}
}
74 changes: 74 additions & 0 deletions src/ZendeskApi.Client/Models/CursorPaginatedIterator.cs
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>
Copy link
Contributor

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?

{

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));
}

}
27 changes: 27 additions & 0 deletions src/ZendeskApi.Client/Models/CursorPaginatedIteratorFactory.cs
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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 ZendeskApi.Client.Pagination or something?

{
public interface ICursorPaginatedIteratorFactory
{
CursorPaginatedIterator<T> Create<T>(ICursorPagination<T> response);
}

public class CursorPaginatedIteratorFactory : ICursorPaginatedIteratorFactory
{
private static IServiceProvider serviceProvider;
Copy link
Contributor

Choose a reason for hiding this comment

The 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 serviceProvider field member should not be static, it should be readonly. We should also not be injecting the IServiceProvder into a class. This endangers running into the Service Locator anti-pattern. I won't go into too much detail here but in short the IoC container is the thing responsible for choosing the correct dependency - in general you don't want a consuming class acting as a locator. The usual pattern for injecting dependencies in dotnet is closer to the following

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

  • alter ICursorPaginatedIteratorFactory:
    • inject IZendeskApiClient directly, instead of the IServiceProvider
    • make the IZendeskApiClient field member private, readonly - not static
  • Register your new interface/implementation in the IoC container. We have helper methods which we recommend using here.

Aside

I 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 IZendeskClient. One approach we already discussed was to change the return type of all CBP endpoints to be the iterator itself. We discounted this because of the blast-radius of the change. It's worth noting that this is probably not that dangerous. As you have explored recently these existing CBP methods don't work properly so people are probably not using it meaningfully. So now is probably the best time to make that change. I do understand your hesitancy though and it may be one for us owners to pick up if you wanted to.

Another approach that works around this is to use the adapter pattern. At the moment the CBP endpoints return ICursorPagination<T>. An adapter class would adapt the adapt the method signature of the CBP methods to return CursorPaginatedIterator<T>. It would work in a similar way to the IteratorFactory you have but the usage is different. You would expose the adapted resources on the IZendeskClient interface so users would consume like so:

client.AdaptedTicketAudit.GetAllAsync()

where the AdaptedTicketAudit is named for clarity but essentially the underlying resource would implement a different interface to ITicketAuditResource

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @mikerogers123, I believe I made the (small) changes you requested 😅
to be very clear, I'm a Ruby developer who picked up this work of updating the C# SDK in order to unblock Zendesk from rolling out the OBP deprecation. How is it looking after this last commit? The usage I wrote in the PR description remains unchanged and I'm registering the Factory here

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Up @@ -13,18 +13,22 @@ public interface IJobStatusResource
/// <summary>
/// Shows the current statuses for background jobs running.
/// </summary>
[Obsolete("Use `GetAllAsync` instead.")]
[Obsolete("Use `GetAllAsync` with CursorPager instead.")]
Task<IPagination<JobStatusResponse>> ListAsync(
PagerParameters pagerParameters = null,
CancellationToken cancellationToken = default(CancellationToken));

/// <summary>
/// Shows the current statuses for background jobs running.
/// </summary>
[Obsolete("Use `GetAllAsync` with CursorPager instead.")]
Task<IPagination<JobStatusResponse>> GetAllAsync(
PagerParameters pagerParameters = null,
CancellationToken cancellationToken = default(CancellationToken));

Task<ICursorPagination<JobStatusResponse>> GetAllAsync(
CursorPager pagerParameters = null,
CancellationToken cancellationToken = default(CancellationToken));
#endregion

#region Show
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ Task<IPagination<Request>> GetAllAsync(
PagerParameters pager = null,
CancellationToken cancellationToken = default);

Task<ICursorPagination<Request>> GetAllAsync(
CursorPager pager,
CancellationToken cancellationToken = default);

Task<Request> GetAsync(
long requestId,
CancellationToken cancellationToken = default);
Expand All @@ -29,6 +33,11 @@ Task<IPagination<TicketComment>> GetAllComments(
PagerParameters pager = null,
CancellationToken cancellationToken = default);

Task<ICursorPagination<TicketComment>> GetAllComments(
long requestId,
CursorPager pager,
CancellationToken cancellationToken = default);

Task<TicketComment> GetTicketCommentAsync(
long requestId,
long commentId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ namespace ZendeskApi.Client.Resources
public interface ITicketAuditResource
{
Task<TicketAuditResponse> GetAllAsync(CursorPagerVariant pager = null, CancellationToken cancellationToken = default);
Task<TicketAuditCursorResponse> GetAllAsync(CursorPager pager, CancellationToken cancellationToken = default);
Task<TicketAuditResponse> GetAllByTicketAsync(long ticketId, CancellationToken cancellationToken = default);
Task<SingleTicketAuditResponse> Get(int ticketId, int auditId, CancellationToken cancellationToken = default);
}
Expand Down
14 changes: 13 additions & 1 deletion src/ZendeskApi.Client/Resources/JobStatusResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public JobStatusResource(
: base(apiClient, logger, "job_statuses")
{ }

[Obsolete("Use `GetAllAsync` instead.")]
[Obsolete("Use `GetAllAsync` with CursorPager instead.")]
public async Task<IPagination<JobStatusResponse>> ListAsync(
PagerParameters pagerParameters = null,
CancellationToken cancellationToken = default(CancellationToken))
Expand All @@ -29,6 +29,18 @@ public async Task<IPagination<JobStatusResponse>> ListAsync(
cancellationToken);
}

public async Task<ICursorPagination<JobStatusResponse>> GetAllAsync(CursorPager pagerParameters, CancellationToken cancellationToken = default)
{
return await GetAsync<JobStatusListCursorResponse>(
ResourceUri,
"list-job-statuses",
"ListAsync",
pagerParameters,
null,
cancellationToken: cancellationToken);
}

[Obsolete("Use `GetAllAsync` with CursorPager instead.")]
public async Task<IPagination<JobStatusResponse>> GetAllAsync(
PagerParameters pagerParameters = null,
CancellationToken cancellationToken = default(CancellationToken))
Expand Down
24 changes: 24 additions & 0 deletions src/ZendeskApi.Client/Resources/RequestsResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,18 @@ public async Task<IPagination<Request>> GetAllAsync(
cancellationToken: cancellationToken);
}

public async Task<ICursorPagination<Request>> GetAllAsync(
CursorPager pager,
CancellationToken cancellationToken = default)
{
return await GetAsync<RequestsCursorResponse>(
ResourceUri,
"list-requests",
"GetAllAsync",
pager,
cancellationToken: cancellationToken);
}


public async Task<Request> GetAsync(
long requestId,
Expand Down Expand Up @@ -79,6 +91,17 @@ public async Task<IPagination<TicketComment>> GetAllComments(
cancellationToken);
}

public async Task<ICursorPagination<TicketComment>> GetAllComments(long requestId, CursorPager pager, CancellationToken cancellationToken = default)
{
return await GetWithNotFoundCheckAsync<TicketCommentsListCursorResponse>(
string.Format(CommentsResourceUri, requestId),
"getting-comments",
$"GetAllComments({requestId})",
$"Could not find any comments for request {requestId} as request was not found",
pager,
cancellationToken);
}

public async Task<TicketComment> GetTicketCommentAsync(
long requestId,
long commentId,
Expand Down Expand Up @@ -121,5 +144,6 @@ public async Task<Request> UpdateAsync(
return response?
.Request;
}

}
}
13 changes: 13 additions & 0 deletions src/ZendeskApi.Client/Resources/Ticket/TicketAuditResource.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
Expand All @@ -17,6 +18,7 @@ public TicketAuditResource(IZendeskApiClient apiClient, ILogger logger) : base(a
{
}

[Obsolete("Use `GetAllAsync` with CursorPager parameter instead.")]
public async Task<TicketAuditResponse> GetAllAsync(CursorPagerVariant pager = null, CancellationToken cancellationToken = default)
{
return await GetAsync<TicketAuditResponse>(
Expand All @@ -27,6 +29,17 @@ public async Task<TicketAuditResponse> GetAllAsync(CursorPagerVariant pager = nu
cancellationToken);
}

public async Task<TicketAuditCursorResponse> GetAllAsync(CursorPager pager, CancellationToken cancellationToken = default)
{
return await GetAsync<TicketAuditCursorResponse>(
ResourceUri,
"list-all-ticket-audits",
"GetAllAsync",
pager,
null,
cancellationToken);
}

public async Task<TicketAuditResponse> GetAllByTicketAsync(long ticketId, CancellationToken cancellationToken = default)
{
return await GetAsync<TicketAuditResponse>(
Expand Down
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;
Expand Down
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;
}
}
Loading