Skip to content

Commit

Permalink
Merge pull request #929 from clement911/Execute-refactor
Browse files Browse the repository at this point in the history 
Refactored core excecution logic to avoid duplication and centralize serialization logic
  • Loading branch information
@clement911
clement911 authored Sep 14, 2023
2 parents c078193 + 40e2e1b commit 4630ec6
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 79 deletions.
3 changes: 0 additions & 3 deletions ShopifySharp.Tests/DateTime_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ public async Task GraphQL_CompareOrderDates()
var order = await this.OrderTestsFixture.Service.GetAsync(orderId);
Assert.NotNull(order.UpdatedAt);

// Shopify seems to take approx. 10 seconds to propagate new orders into the GraphQL API
await Task.Delay(TimeSpan.FromSeconds(11));

var graphService = new GraphService(Utils.MyShopifyUrl, Utils.AccessToken);
graphService.SetExecutionPolicy(new LeakyBucketExecutionPolicy());
var graphQlOrder = await graphService.PostAsync(
Expand Down
31 changes: 19 additions & 12 deletions ShopifySharp/Infrastructure/Serializer.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using ShopifySharp.Converters;
using System;
using System.Collections.Generic;

namespace ShopifySharp.Infrastructure
Expand All @@ -13,27 +12,35 @@ public static class Serializer
{
public static JsonSerializerSettings CreateSettings()
{
return new JsonSerializerSettings
return new JsonSerializerSettings
{
DateParseHandling = DateParseHandling.DateTimeOffset,
NullValueHandling = NullValueHandling.Ignore,
Converters = new List<JsonConverter>
{
NullValueHandling = NullValueHandling.Ignore,
Converters = new List<JsonConverter>
{
new InvalidDateConverter()
}
};
}

public static string Serialize(object data) => JsonConvert.SerializeObject(data, CreateSettings());

public static object Deserialize(string json, Type objectType) => JsonConvert.DeserializeObject(json, objectType, CreateSettings());

public static T Deserialize<T>(string json) => JsonConvert.DeserializeObject<T>(json, CreateSettings());

public static T Deserialize<T>(string json, string rootElementPath)
public static T Deserialize<T>(string json, string rootElementPath = null, DateParseHandling? dateParseHandlingOverride = null)
{
var jToken = Deserialize<JToken>(json).SelectToken(rootElementPath);
return jToken.ToObject<T>(JsonSerializer.Create(CreateSettings()));
var settings = CreateSettings();
if (dateParseHandlingOverride != null)
settings.DateParseHandling = dateParseHandlingOverride.Value;

if (rootElementPath != null)
{
var jToken = JsonConvert.DeserializeObject<JToken>(json, settings);
jToken = jToken.SelectToken(rootElementPath);
return jToken.ToObject<T>(JsonSerializer.Create(settings));
}
else
{
return JsonConvert.DeserializeObject<T>(json, settings);
}
}
}
}
2 changes: 1 addition & 1 deletion ShopifySharp/Services/Graph/GraphService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public virtual async Task<JToken> PostAsync(JToken body, int? graphqlQueryCost =
/// <returns>A JToken containing the data from the request.</returns>
protected virtual async Task<JToken> SendAsync(RequestUri req, HttpContent content, int? graphqlQueryCost, CancellationToken cancellationToken = default)
{
var response = await ExecuteRequestAsync(req, HttpMethod.Post, cancellationToken, content, graphqlQueryCost: graphqlQueryCost);
var response = await ExecuteRequestAsync(req, HttpMethod.Post, cancellationToken, content, null, graphqlQueryCost, DateParseHandling.None);

CheckForErrors(response);

Expand Down
108 changes: 46 additions & 62 deletions ShopifySharp/Services/ShopifyService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,14 @@ private string ReadLinkHeader(HttpResponseMessage response)
return linkHeaderValues == null ? null : string.Join(", ", linkHeaderValues);
}

/// <summary>
/// Executes a request and returns a JToken for querying. Throws an exception when the response is invalid.
/// Use this method when the expected response is a single line or simple object that doesn't warrant its own class.
/// </summary>
/// <remarks>
/// This method will automatically dispose the <paramref name="baseClient"/> and <paramref name="content" /> when finished.
/// </remarks>
protected async Task<RequestResult<JToken>> ExecuteRequestAsync(RequestUri uri, HttpMethod method,
CancellationToken cancellationToken, HttpContent content = null, Dictionary<string, string> headers = null, int? graphqlQueryCost = null)
protected async Task<RequestResult<T>> ExecuteRequestCoreAsync<T>(RequestUri uri,
HttpMethod method,
CancellationToken cancellationToken,
HttpContent content,
Dictionary<string, string> headers,
string rootElement,
int? graphqlQueryCost,
DateParseHandling? dateParseHandlingOverride = null)
{
using (var baseRequestMessage = PrepareRequestMessage(uri, method, content, headers))
{
Expand All @@ -185,64 +184,49 @@ protected async Task<RequestResult<JToken>> ExecuteRequestAsync(RequestUri uri,
//Check for and throw exception when necessary.
CheckResponseExceptions(response, rawResult);
JToken jtoken = null;
var result = method == HttpMethod.Delete ? default : Serializer.Deserialize<T>(rawResult, rootElement, dateParseHandlingOverride);
// Don't parse the result when the request was Delete.
if (baseRequestMessage.Method != HttpMethod.Delete)
{
// Make sure that dates are not stripped of any timezone information if tokens are de-serialised into strings/DateTime/DateTimeZoneOffset
using (var reader = new JsonTextReader(new StringReader(rawResult)) { DateParseHandling = DateParseHandling.None })
{
jtoken = await JObject.LoadAsync(reader, cancellationToken);
}
}
return new RequestResult<JToken>(response, jtoken, rawResult, ReadLinkHeader(response));
return new RequestResult<T>(response, result, rawResult, ReadLinkHeader(response));
}
}, cancellationToken, graphqlQueryCost);

return policyResult;
}
}

/// <summary>
/// Executes a request and returns a JToken for querying. Throws an exception when the response is invalid.
/// Use this method when the expected response is a single line or simple object that doesn't warrant its own class.
/// </summary>
/// <remarks>
/// This method will automatically dispose the <paramref name="baseClient"/> and <paramref name="content" /> when finished.
/// </remarks>
protected async Task<RequestResult<JToken>> ExecuteRequestAsync(RequestUri uri,
HttpMethod method,
CancellationToken cancellationToken,
HttpContent content = null,
Dictionary<string, string> headers = null,
int? graphqlQueryCost = null,
DateParseHandling? dateParseHandlingOverride = null)
{
return await this.ExecuteRequestCoreAsync<JToken>(uri, method, cancellationToken, content, headers, null, graphqlQueryCost, dateParseHandlingOverride);
}

/// <summary>
/// Executes a request and returns the given type. Throws an exception when the response is invalid.
/// Use this method when the expected response is a single line or simple object that doesn't warrant its own class.
/// </summary>
/// <remarks>
/// This method will automatically dispose the <paramref name="baseRequestMessage" /> when finished.
/// </remarks>
protected async Task<RequestResult<T>> ExecuteRequestAsync<T>(RequestUri uri, HttpMethod method,
CancellationToken cancellationToken, HttpContent content = null, string rootElement = null, Dictionary<string, string> headers = null)
protected async Task<RequestResult<T>> ExecuteRequestAsync<T>(RequestUri uri,
HttpMethod method,
CancellationToken cancellationToken,
HttpContent content = null,
string rootElement = null,
Dictionary<string, string> headers = null)
{
using (var baseRequestMessage = PrepareRequestMessage(uri, method, content, headers))
{
var policyResult = await _ExecutionPolicy.Run(baseRequestMessage, async (requestMessage) =>
{
var request = _Client.SendAsync(requestMessage, cancellationToken);
using (var response = await request)
{
var rawResult = await response.Content.ReadAsStringAsync();
//Check for and throw exception when necessary.
CheckResponseExceptions(response, rawResult);
T result = default;
if (rootElement != null)
{
// This method may fail when the method was Delete, which is intendend.
// Delete methods should not be parsing the response JSON and should instead
// be using the non-generic ExecuteRequestAsync.
result = Serializer.Deserialize<T>(rawResult, rootElement);
}
return new RequestResult<T>(response, result, rawResult, ReadLinkHeader(response));
}
}, cancellationToken);

return policyResult;
}
return await this.ExecuteRequestCoreAsync<T>(uri, method, cancellationToken, content, headers, rootElement, null);
}

private async Task<T> ExecuteWithContentCoreAsync<T>(string path, string resultRootElt, HttpMethod method, JsonContent content, CancellationToken cancellationToken)
Expand Down Expand Up @@ -325,7 +309,7 @@ public static void CheckResponseExceptions(HttpResponseMessage response, string
{
string rateExceptionMessage;
IEnumerable<string> errors;

if (TryParseErrorJson(rawResponse, out var rateLimitErrors))
{
rateExceptionMessage = $"({statusMessage}) {rateLimitErrors.First()}";
Expand All @@ -335,7 +319,7 @@ public static void CheckResponseExceptions(HttpResponseMessage response, string
{
var baseMessage = "Exceeded the rate limit for api client. Reduce request rates to resume uninterrupted service.";
rateExceptionMessage = $"({statusMessage}) {baseMessage}";
errors = new List<string>{ baseMessage };
errors = new List<string> { baseMessage };
}

throw new ShopifyRateLimitException(response, code, errors, rateExceptionMessage, rawResponse, requestId);
Expand All @@ -356,19 +340,19 @@ public static void CheckResponseExceptions(HttpResponseMessage response, string

switch (totalErrors)
{
case 1 :
case 1:
exceptionMessage = baseErrorMessage;
break;

case 2:
exceptionMessage = $"{baseErrorMessage} (and one other error)";
break;

default:
exceptionMessage = $"{baseErrorMessage} (and {totalErrors} other errors)";
break;
}

errors = parsedErrors;
}
else
Expand Down Expand Up @@ -398,7 +382,7 @@ public static void CheckResponseExceptions(HttpResponseMessage response, string
public static bool TryParseErrorJson(string json, out List<string> output)
{
output = null;

if (string.IsNullOrEmpty(json))
{
return false;
Expand Down Expand Up @@ -427,14 +411,14 @@ public static bool TryParseErrorJson(string json, out List<string> output)
// Error is type #4
var description = parsed["error_description"].Value<string>();
var errorType = parsed["error"].Value<string>();

errors.Add($"{errorType}: {description}");
}
else if (parsed.Any(p => p.Path == "error"))
{
// Error is type #5
var description = parsed["error"].Value<string>();

errors.Add(description);
}
else if (parsed.Any(x => x.Path == "errors"))
Expand All @@ -459,7 +443,7 @@ public static bool TryParseErrorJson(string json, out List<string> output)
if (val.Type == JTokenType.String)
{
var description = val.Value<string>();

errors.Add($"{name}: {description}");
}
else if (val.Type == JTokenType.Array)
Expand Down Expand Up @@ -488,7 +472,7 @@ public static bool TryParseErrorJson(string json, out List<string> output)
}

output = errors;

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion ShopifySharp/ShopifySharp.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<TargetFrameworks>netstandard2.0</TargetFrameworks>
<AssemblyName>ShopifySharp</AssemblyName>
<RootNamespace>ShopifySharp</RootNamespace>
<VersionPrefix>6.3.3</VersionPrefix>
<VersionPrefix>6.4.0</VersionPrefix>
<PublishRepositoryUrl>true</PublishRepositoryUrl>
<IncludeSymbols>true</IncludeSymbols>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
Expand Down

0 comments on commit 4630ec6

Please sign in to comment.