From 03c3acbe23ca9bd57615ffe64f0d1e57db69e0f7 Mon Sep 17 00:00:00 2001 From: Euan Torano Date: Wed, 27 Sep 2023 08:20:16 +0100 Subject: [PATCH] Ref #176 - Added support for cancellation tokens. Fixed #130 - await rather than accessing task result. --- .../GovukNotify.Tests.csproj | 12 +-- src/GovukNotify/Client/BaseClient.cs | 76 ++++++++++++------- src/GovukNotify/Client/HttpClientWrapper.cs | 27 ++++++- src/GovukNotify/Client/NotificationClient.cs | 72 +++++++++++------- src/GovukNotify/GovukNotify.csproj | 2 + .../Interfaces/IAsyncNotificationClient.cs | 23 +++--- src/GovukNotify/Interfaces/IBaseClient.cs | 7 +- src/GovukNotify/Interfaces/IHttpClient.cs | 3 +- 8 files changed, 141 insertions(+), 81 deletions(-) diff --git a/src/GovukNotify.Tests/GovukNotify.Tests.csproj b/src/GovukNotify.Tests/GovukNotify.Tests.csproj index 46f3adc..0bf0e7c 100644 --- a/src/GovukNotify.Tests/GovukNotify.Tests.csproj +++ b/src/GovukNotify.Tests/GovukNotify.Tests.csproj @@ -7,12 +7,12 @@ - - - - - - + + + + + + diff --git a/src/GovukNotify/Client/BaseClient.cs b/src/GovukNotify/Client/BaseClient.cs index be32381..5074d00 100644 --- a/src/GovukNotify/Client/BaseClient.cs +++ b/src/GovukNotify/Client/BaseClient.cs @@ -7,6 +7,7 @@ using System.Net.Http; using System.Reflection; using System.Text; +using System.Threading; using System.Threading.Tasks; namespace Notify.Client @@ -34,55 +35,71 @@ public BaseClient(IHttpClient client, string apiKey, string baseUrl = "https://a this.client.BaseAddress = ValidateBaseUri(BaseUrl); this.client.AddContentHeader("application/json"); - var productVersion = typeof(BaseClient).GetTypeInfo().Assembly.GetName().Version.ToString(); + var productVersion = typeof(BaseClient).GetTypeInfo().Assembly.GetName().Version?.ToString(); this.client.AddUserAgent(NOTIFY_CLIENT_NAME + productVersion); } - public async Task GET(string url) + public async Task GET(string url, CancellationToken cancellationToken = default) { - return await MakeRequest(url, HttpMethod.Get).ConfigureAwait(false); + return await MakeRequest(url, HttpMethod.Get, cancellationToken: cancellationToken) + .ConfigureAwait(false); } - public async Task POST(string url, string json) + public async Task POST(string url, string json, CancellationToken cancellationToken = default) { - var content = new StringContent(json, Encoding.UTF8, "application/json"); - return await MakeRequest(url, HttpMethod.Post, content).ConfigureAwait(false); + using var content = new StringContent(json, Encoding.UTF8, "application/json"); + return await MakeRequest(url, HttpMethod.Post, content, cancellationToken: cancellationToken) + .ConfigureAwait(false); } - public async Task GETBytes(string url) + public async Task GETBytes(string url, CancellationToken cancellationToken = default) { - return await MakeRequestBytes(url, HttpMethod.Get).ConfigureAwait(false); + return await MakeRequestBytes(url, HttpMethod.Get, cancellationToken: cancellationToken) + .ConfigureAwait(false); } - public async Task MakeRequestBytes(string url, HttpMethod method, HttpContent content = null) { - var response = SendRequest(url, method, content).Result; - - var responseContent = await response.Content.ReadAsByteArrayAsync().ConfigureAwait(false); - + public async Task MakeRequestBytes(string url, HttpMethod method, HttpContent content = null, CancellationToken cancellationToken = default) { + var response = await SendRequest(url, method, content, cancellationToken); + +#if NET6_0_OR_GREATER + var responseContent = await response.Content.ReadAsByteArrayAsync(cancellationToken) + .ConfigureAwait(false); +#else + var responseContent = await response.Content.ReadAsByteArrayAsync() + .ConfigureAwait(false); +#endif + if (!response.IsSuccessStatusCode) { // if there was an error, rather than a binary pdf, the http body will be a json error message, so // encode the bytes as UTF8 - HandleHTTPErrors(response, Encoding.UTF8.GetString(responseContent)); + BaseClient.HandleHTTPErrors(response, Encoding.UTF8.GetString(responseContent)); } - return responseContent; + return responseContent; } - public async Task MakeRequest(string url, HttpMethod method, HttpContent content = null) + public async Task MakeRequest(string url, HttpMethod method, HttpContent content = null, CancellationToken cancellationToken = default) { - var response = SendRequest(url, method, content).Result; - - var responseContent = await response.Content.ReadAsStringAsync().ConfigureAwait(false); - + var response = await SendRequest(url, method, content, cancellationToken); + +#if NET6_0_OR_GREATER + var responseContent = await response.Content.ReadAsStringAsync(cancellationToken) + .ConfigureAwait(false); +#else + var responseContent = await response.Content.ReadAsStringAsync() + .ConfigureAwait(false); +#endif + if (!response.IsSuccessStatusCode) { HandleHTTPErrors(response, responseContent); } + return responseContent; } - private async Task SendRequest(string url, HttpMethod method, HttpContent content) + private async Task SendRequest(string url, HttpMethod method, HttpContent content, CancellationToken cancellationToken = default) { var request = new HttpRequestMessage(method, url); @@ -98,7 +115,7 @@ private async Task SendRequest(string url, HttpMethod metho try { - response = await this.client.SendAsync(request).ConfigureAwait(false); + response = await this.client.SendAsync(request, cancellationToken).ConfigureAwait(false); } catch (AggregateException ae) { @@ -106,7 +123,7 @@ private async Task SendRequest(string url, HttpMethod metho { if (x is HttpRequestException) { - throw new NotifyClientException(x.InnerException.Message); + throw new NotifyClientException(x.InnerException?.Message ?? x.Message); } throw x; }); @@ -115,7 +132,7 @@ private async Task SendRequest(string url, HttpMethod metho return response; } - private void HandleHTTPErrors(HttpResponseMessage response, string errorResponseContent) + private static void HandleHTTPErrors(HttpResponseMessage response, string errorResponseContent) { try { @@ -129,8 +146,12 @@ private void HandleHTTPErrors(HttpResponseMessage response, string errorResponse } public Tuple ExtractServiceIdAndApiKey(string fromApiKey) - { - if (fromApiKey.Length < 74 || string.IsNullOrWhiteSpace(fromApiKey) || fromApiKey.Contains(" ")) + { +#if NET6_0_OR_GREATER + if (string.IsNullOrWhiteSpace(fromApiKey) || fromApiKey.Length < 74 || fromApiKey.Contains(' ')) +#else + if (string.IsNullOrWhiteSpace(fromApiKey) || fromApiKey.Length < 74 || fromApiKey.Contains(" ")) +#endif { throw new NotifyAuthException("The API Key provided is invalid. Please ensure you are using a v2 API Key that is not empty or null"); } @@ -144,7 +165,7 @@ public Tuple ExtractServiceIdAndApiKey(string fromApiKey) public Uri ValidateBaseUri(string baseUrl) { var result = Uri.TryCreate(baseUrl, UriKind.Absolute, out var uriResult) - && (uriResult.Scheme == "http" || uriResult.Scheme == "https"); + && uriResult.Scheme is "http" or "https"; if (!result) { @@ -152,7 +173,6 @@ public Uri ValidateBaseUri(string baseUrl) } return uriResult; - } public string GetUserAgent() diff --git a/src/GovukNotify/Client/HttpClientWrapper.cs b/src/GovukNotify/Client/HttpClientWrapper.cs index c979e10..335a729 100644 --- a/src/GovukNotify/Client/HttpClientWrapper.cs +++ b/src/GovukNotify/Client/HttpClientWrapper.cs @@ -2,6 +2,7 @@ using System; using System.Net.Http; using System.Net.Http.Headers; +using System.Threading; using System.Threading.Tasks; namespace Notify.Client @@ -9,8 +10,11 @@ namespace Notify.Client public class HttpClientWrapper : IHttpClient { private readonly HttpClient _client; + private Uri _baseAddress; + private bool _disposed; + public Uri BaseAddress { get => _baseAddress; @@ -28,12 +32,29 @@ public HttpClientWrapper(HttpClient client) public void Dispose() { - _client.Dispose(); + Dispose(true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) + { + if (_disposed) + { + return; + } + + if (disposing) + { + _client.Dispose(); + } + + _disposed = true; } - public async Task SendAsync(HttpRequestMessage request) + public async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default) { - return await _client.SendAsync(request).ConfigureAwait(false); + return await _client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken) + .ConfigureAwait(false); } public void SetClientBaseAddress() diff --git a/src/GovukNotify/Client/NotificationClient.cs b/src/GovukNotify/Client/NotificationClient.cs index d9223e8..a14f98d 100644 --- a/src/GovukNotify/Client/NotificationClient.cs +++ b/src/GovukNotify/Client/NotificationClient.cs @@ -7,10 +7,10 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; -using System.IO; using System.Linq; using System.Net; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; namespace Notify.Client @@ -42,11 +42,12 @@ public NotificationClient(IHttpClient client, string apiKey) : base(client, apiK { } - public async Task GetNotificationByIdAsync(string notificationId) + public async Task GetNotificationByIdAsync(string notificationId, CancellationToken cancellationToken = default) { var url = GET_NOTIFICATION_URL + notificationId; - var response = await this.GET(url).ConfigureAwait(false); + var response = await this.GET(url, cancellationToken) + .ConfigureAwait(false); try { @@ -72,7 +73,7 @@ select string.Format("{0}={1}", } public async Task GetNotificationsAsync(string templateType = "", string status = "", string reference = "", - string olderThanId = "", bool includeSpreadsheetUploads = false) + string olderThanId = "", bool includeSpreadsheetUploads = false, CancellationToken cancellationToken = default) { var query = new NameValueCollection(); if (!string.IsNullOrWhiteSpace(templateType)) @@ -101,13 +102,14 @@ public async Task GetNotificationsAsync(string templateType = } var finalUrl = GET_ALL_NOTIFICATIONS_URL + ToQueryString(query); - var response = await GET(finalUrl).ConfigureAwait(false); + var response = await GET(finalUrl, cancellationToken) + .ConfigureAwait(false); var notifications = JsonConvert.DeserializeObject(response); return notifications; } - public async Task GetAllTemplatesAsync(string templateType = "") + public async Task GetAllTemplatesAsync(string templateType = "", CancellationToken cancellationToken = default) { var finalUrl = string.Format( "{0}{1}", @@ -115,14 +117,15 @@ public async Task GetAllTemplatesAsync(string templateType = "") templateType == string.Empty ? string.Empty : TYPE_PARAM + templateType ); - var response = await GET(finalUrl).ConfigureAwait(false); + var response = await GET(finalUrl, cancellationToken) + .ConfigureAwait(false); var templateList = JsonConvert.DeserializeObject(response); return templateList; } - public async Task GetReceivedTextsAsync(string olderThanId = "") + public async Task GetReceivedTextsAsync(string olderThanId = "", CancellationToken cancellationToken = default) { var finalUrl = string.Format( "{0}{1}", @@ -130,7 +133,8 @@ public async Task GetReceivedTextsAsync(string olderTh string.IsNullOrWhiteSpace(olderThanId) ? "" : "?older_than=" + olderThanId ); - var response = await this.GET(finalUrl).ConfigureAwait(false); + var response = await this.GET(finalUrl, cancellationToken) + .ConfigureAwait(false); var receivedTexts = JsonConvert.DeserializeObject(response); @@ -139,7 +143,7 @@ public async Task GetReceivedTextsAsync(string olderTh public async Task SendSmsAsync(string mobileNumber, string templateId, Dictionary personalisation = null, string clientReference = null, - string smsSenderId = null) + string smsSenderId = null, CancellationToken cancellationToken = default) { var o = CreateRequestParams(templateId, personalisation, clientReference); o.AddFirst(new JProperty("phone_number", mobileNumber)); @@ -149,14 +153,15 @@ public async Task SendSmsAsync(string mobileNumber, str o.Add(new JProperty("sms_sender_id", smsSenderId)); } - var response = await POST(SEND_SMS_NOTIFICATION_URL, o.ToString(Formatting.None)).ConfigureAwait(false); + var response = await POST(SEND_SMS_NOTIFICATION_URL, o.ToString(Formatting.None), cancellationToken) + .ConfigureAwait(false); return JsonConvert.DeserializeObject(response); } public async Task SendEmailAsync(string emailAddress, string templateId, Dictionary personalisation = null, string clientReference = null, - string emailReplyToId = null) + string emailReplyToId = null, CancellationToken cancellationToken = default) { var o = CreateRequestParams(templateId, personalisation, clientReference); o.AddFirst(new JProperty("email_address", emailAddress)); @@ -166,22 +171,25 @@ public async Task SendEmailAsync(string emailAddress, o.Add(new JProperty("email_reply_to_id", emailReplyToId)); } - var response = await POST(SEND_EMAIL_NOTIFICATION_URL, o.ToString(Formatting.None)).ConfigureAwait(false); + var response = await POST(SEND_EMAIL_NOTIFICATION_URL, o.ToString(Formatting.None), cancellationToken) + .ConfigureAwait(false); return JsonConvert.DeserializeObject(response); } public async Task SendLetterAsync(string templateId, Dictionary personalisation, - string clientReference = null) + string clientReference = null, CancellationToken cancellationToken = default) { var o = CreateRequestParams(templateId, personalisation, clientReference); - var response = await this.POST(SEND_LETTER_NOTIFICATION_URL, o.ToString(Formatting.None)).ConfigureAwait(false); + var response = await this.POST(SEND_LETTER_NOTIFICATION_URL, o.ToString(Formatting.None), cancellationToken) + .ConfigureAwait(false); return JsonConvert.DeserializeObject(response); } - public async Task SendPrecompiledLetterAsync(string clientReference, byte[] pdfContents, string postage = null) + public async Task SendPrecompiledLetterAsync(string clientReference, byte[] pdfContents, + string postage = null, CancellationToken cancellationToken = default) { var requestParams = new JObject { @@ -194,28 +202,31 @@ public async Task SendPrecompiledLetterAsync(string requestParams.Add(new JProperty("postage", postage)); } - var response = await this.POST(SEND_LETTER_NOTIFICATION_URL, requestParams.ToString(Formatting.None)).ConfigureAwait(false); + var response = await this.POST(SEND_LETTER_NOTIFICATION_URL, requestParams.ToString(Formatting.None), cancellationToken) + .ConfigureAwait(false); return JsonConvert.DeserializeObject(response); } - public async Task GetTemplateByIdAsync(string templateId) + public async Task GetTemplateByIdAsync(string templateId, CancellationToken cancellationToken = default) { var url = GET_TEMPLATE_URL + templateId; - return await GetTemplateFromURLAsync(url).ConfigureAwait(false); + return await GetTemplateFromURLAsync(url, cancellationToken) + .ConfigureAwait(false); } - public async Task GetTemplateByIdAndVersionAsync(string templateId, int version = 0) + public async Task GetTemplateByIdAndVersionAsync(string templateId, int version = 0, CancellationToken cancellationToken = default) { var pattern = "{0}{1}" + (version > 0 ? VERSION_PARAM + "{2}" : ""); var url = string.Format(pattern, GET_TEMPLATE_URL, templateId, version); - return await GetTemplateFromURLAsync(url).ConfigureAwait(false); + return await GetTemplateFromURLAsync(url, cancellationToken) + .ConfigureAwait(false); } public async Task GenerateTemplatePreviewAsync(string templateId, - Dictionary personalisation = null) + Dictionary personalisation = null, CancellationToken cancellationToken = default) { var url = string.Format("{0}{1}/preview", GET_TEMPLATE_URL, templateId); @@ -224,7 +235,8 @@ public async Task GenerateTemplatePreviewAsync(string t {"personalisation", JObject.FromObject(personalisation)} }; - var response = await this.POST(url, o.ToString(Formatting.None)).ConfigureAwait(false); + var response = await this.POST(url, o.ToString(Formatting.None), cancellationToken) + .ConfigureAwait(false); try { @@ -238,10 +250,11 @@ public async Task GenerateTemplatePreviewAsync(string t } - public async Task GetPdfForLetterAsync(string notificationId) + public async Task GetPdfForLetterAsync(string notificationId, CancellationToken cancellationToken = default) { var finalUrl = string.Format(GET_PDF_FOR_LETTER_URL, notificationId); - var response = await GETBytes(finalUrl).ConfigureAwait(false); + var response = await GETBytes(finalUrl, cancellationToken) + .ConfigureAwait(false); return response; } @@ -273,9 +286,10 @@ public static JObject PrepareUpload(byte[] documentContents, bool isCsv = false) }; } - private async Task GetTemplateFromURLAsync(string url) + private async Task GetTemplateFromURLAsync(string url, CancellationToken cancellationToken = default) { - var response = await this.GET(url).ConfigureAwait(false); + var response = await this.GET(url, cancellationToken) + .ConfigureAwait(false); try { @@ -316,10 +330,10 @@ private static Exception HandleAggregateException(AggregateException ex) { if (ex == null) { - throw new ArgumentNullException("ex"); + throw new ArgumentNullException(nameof(ex)); } - if (ex.InnerExceptions != null && ex.InnerExceptions.Count == 1) + if (ex.InnerExceptions.Count == 1) { return ex.InnerException; } diff --git a/src/GovukNotify/GovukNotify.csproj b/src/GovukNotify/GovukNotify.csproj index 6c2570e..14b869f 100644 --- a/src/GovukNotify/GovukNotify.csproj +++ b/src/GovukNotify/GovukNotify.csproj @@ -8,6 +8,8 @@ GOV.UK .NET Notify Client false See https://github.com/alphagov/notifications-net-client/blob/main/CHANGELOG.md + Notify + latest diff --git a/src/GovukNotify/Interfaces/IAsyncNotificationClient.cs b/src/GovukNotify/Interfaces/IAsyncNotificationClient.cs index c42e5e7..6a615f4 100644 --- a/src/GovukNotify/Interfaces/IAsyncNotificationClient.cs +++ b/src/GovukNotify/Interfaces/IAsyncNotificationClient.cs @@ -1,32 +1,33 @@ using Notify.Models; using Notify.Models.Responses; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; namespace Notify.Interfaces { public interface IAsyncNotificationClient : IBaseClient { - Task GenerateTemplatePreviewAsync(string templateId, Dictionary personalisation = null); + Task GenerateTemplatePreviewAsync(string templateId, Dictionary personalisation = null, CancellationToken cancellationToken = default); - Task GetAllTemplatesAsync(string templateType = ""); + Task GetAllTemplatesAsync(string templateType = "", CancellationToken cancellationToken = default); - Task GetNotificationByIdAsync(string notificationId); + Task GetNotificationByIdAsync(string notificationId, CancellationToken cancellationToken = default); - Task GetNotificationsAsync(string templateType = "", string status = "", string reference = "", string olderThanId = "", bool includeSpreadsheetUploads = false); + Task GetNotificationsAsync(string templateType = "", string status = "", string reference = "", string olderThanId = "", bool includeSpreadsheetUploads = false, CancellationToken cancellationToken = default); - Task GetReceivedTextsAsync(string olderThanId = ""); + Task GetReceivedTextsAsync(string olderThanId = "", CancellationToken cancellationToken = default); - Task GetTemplateByIdAsync(string templateId); + Task GetTemplateByIdAsync(string templateId, CancellationToken cancellationToken = default); - Task GetTemplateByIdAndVersionAsync(string templateId, int version = 0); + Task GetTemplateByIdAndVersionAsync(string templateId, int version = 0, CancellationToken cancellationToken = default); - Task SendSmsAsync(string mobileNumber, string templateId, Dictionary personalisation = null, string clientReference = null, string smsSenderId = null); + Task SendSmsAsync(string mobileNumber, string templateId, Dictionary personalisation = null, string clientReference = null, string smsSenderId = null, CancellationToken cancellationToken = default); - Task SendEmailAsync(string emailAddress, string templateId, Dictionary personalisation = null, string clientReference = null, string emailReplyToId = null); + Task SendEmailAsync(string emailAddress, string templateId, Dictionary personalisation = null, string clientReference = null, string emailReplyToId = null, CancellationToken cancellationToken = default); - Task SendLetterAsync(string templateId, Dictionary personalisation, string clientReference = null); + Task SendLetterAsync(string templateId, Dictionary personalisation, string clientReference = null, CancellationToken cancellationToken = default); - Task SendPrecompiledLetterAsync(string clientReference, byte[] pdfContents, string postage); + Task SendPrecompiledLetterAsync(string clientReference, byte[] pdfContents, string postage, CancellationToken cancellationToken = default); } } diff --git a/src/GovukNotify/Interfaces/IBaseClient.cs b/src/GovukNotify/Interfaces/IBaseClient.cs index 590dd82..064dade 100644 --- a/src/GovukNotify/Interfaces/IBaseClient.cs +++ b/src/GovukNotify/Interfaces/IBaseClient.cs @@ -1,16 +1,17 @@ using System; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; namespace Notify.Interfaces { public interface IBaseClient { - Task GET(string url); + Task GET(string url, CancellationToken cancellationToken = default); - Task POST(string url, string json); + Task POST(string url, string json, CancellationToken cancellationToken = default); - Task MakeRequest(string url, HttpMethod method, HttpContent content = null); + Task MakeRequest(string url, HttpMethod method, HttpContent content = null, CancellationToken cancellationToken = default); Tuple ExtractServiceIdAndApiKey(string fromApiKey); diff --git a/src/GovukNotify/Interfaces/IHttpClient.cs b/src/GovukNotify/Interfaces/IHttpClient.cs index bd1b64a..dc33e07 100644 --- a/src/GovukNotify/Interfaces/IHttpClient.cs +++ b/src/GovukNotify/Interfaces/IHttpClient.cs @@ -1,12 +1,13 @@ using System; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; namespace Notify.Interfaces { public interface IHttpClient : IDisposable { - Task SendAsync(HttpRequestMessage request); + Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default); Uri BaseAddress { get; set; }