Skip to content

Commit

Permalink
addressed PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
bachuv committed Nov 10, 2023
1 parent de6a60b commit d0e6e99
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 86 deletions.
22 changes: 6 additions & 16 deletions src/Worker.Extensions.DurableTask/HTTP/DurableHttpRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,10 @@ public class DurableHttpRequest
/// </summary>
public DurableHttpRequest(
HttpMethod method,
Uri uri,
IDictionary<string, StringValues>? headers = null,
string? content = null,
bool asynchronousPatternEnabled = true,
TimeSpan? timeout = null,
HttpRetryOptions? httpRetryOptions = null)
Uri uri)
{
this.Method = method;
this.Uri = uri;
this.Headers = headers;
this.Content = content;
this.AsynchronousPatternEnabled = asynchronousPatternEnabled;
this.Timeout = timeout;
this.HttpRetryOptions = httpRetryOptions;
}

/// <summary>
Expand All @@ -53,32 +43,32 @@ public DurableHttpRequest(
/// </summary>
[JsonPropertyName("headers")]
[JsonConverter(typeof(HttpHeadersConverter))]
public IDictionary<string, StringValues>? Headers { get; }
public IDictionary<string, StringValues>? Headers { get; set; }

/// <summary>
/// Content passed with the HTTP request made by the Durable Function.
/// </summary>
[JsonPropertyName("content")]
public string? Content { get; }
public string? Content { get; set; }

/// <summary>
/// Specifies whether the Durable HTTP APIs should automatically
/// handle the asynchronous HTTP pattern.
/// </summary>
[JsonPropertyName("asynchronousPatternEnabled")]
public bool AsynchronousPatternEnabled { get; }
public bool AsynchronousPatternEnabled { get; set; }

/// <summary>
/// Defines retry policy for handling of failures in making the HTTP Request. These could be non-successful HTTP status codes
/// in the response, a timeout in making the HTTP call, or an exception raised from the HTTP Client library.
/// </summary>
[JsonPropertyName("retryOptions")]
public HttpRetryOptions? HttpRetryOptions { get; }
public HttpRetryOptions? HttpRetryOptions { get; set; }

/// <summary>
/// The total timeout for the original HTTP request and any
/// asynchronous polling.
/// </summary>
[JsonPropertyName("timeout")]
public TimeSpan? Timeout { get; }
public TimeSpan? Timeout { get; set; }
}
12 changes: 3 additions & 9 deletions src/Worker.Extensions.DurableTask/HTTP/DurableHttpResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,10 @@ public class DurableHttpResponse
/// Initializes a new instance of the <see cref="DurableHttpResponse"/> class.
/// </summary>
/// <param name="statusCode">HTTP Status code returned from the HTTP call.</param>
/// <param name="headers">Headers returned from the HTTP call.</param>
/// <param name="content">Content returned from the HTTP call.</param>
public DurableHttpResponse(
HttpStatusCode statusCode,
IDictionary<string, StringValues>? headers = null,
string? content = null)
HttpStatusCode statusCode)
{
this.StatusCode = statusCode;
this.Headers = headers;
this.Content = content;
}

/// <summary>
Expand All @@ -40,11 +34,11 @@ public DurableHttpResponse(
/// </summary>
[JsonPropertyName("headers")]
[JsonConverter(typeof(HttpHeadersConverter))]
public IDictionary<string, StringValues>? Headers { get; }
public IDictionary<string, StringValues>? Headers { get; init; }

/// <summary>
/// Content returned from an HTTP request.
/// </summary>
[JsonPropertyName("content")]
public string? Content { get; }
public string? Content { get; init; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ namespace Microsoft.Azure.Functions.Worker.Extensions.DurableTask.Http;
// for serializing HTTP header collections
internal class HttpHeadersConverter : JsonConverter<IDictionary<string, StringValues>>
{
public override bool CanConvert(Type objectType)
{
return typeof(IDictionary<string, StringValues>).IsAssignableFrom(objectType);
}

public override IDictionary<string, StringValues> Read(
ref Utf8JsonReader reader,
Type objectType,
Expand Down
13 changes: 1 addition & 12 deletions src/Worker.Extensions.DurableTask/HTTP/HttpMethodConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,7 @@ public override HttpMethod Read(
Type objectType,
JsonSerializerOptions options)
{
HttpMethod method = HttpMethod.Get;

if (reader.TokenType != JsonTokenType.StartObject)
{
return method;
}

reader.Read();

method = new HttpMethod(reader.GetString());

return method;
return new HttpMethod(reader.GetString());
}

public override void Write(
Expand Down
35 changes: 7 additions & 28 deletions src/Worker.Extensions.DurableTask/HTTP/HttpRetryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,13 @@ public class HttpRetryOptions
// facing type, that is difficult.
private static readonly TimeSpan DefaultMaxRetryinterval = TimeSpan.FromDays(6);

private TimeSpan firstRetryInterval;
private int maxNumberOfAttempts;
private IList<HttpStatusCode>? statusCodesToRetry;

/// <summary>
/// Creates a new instance SerializableRetryOptions with the supplied first retry and max attempts.
/// </summary>
/// <param name="firstRetryInterval">Timespan to wait for the first retry.</param>
/// <param name="maxNumberOfAttempts">Max number of attempts to retry.</param>
/// <param name="statusCodesToRetry">List of status codes that specify when to retry.</param>
/// <exception cref="ArgumentException">
/// The <paramref name="firstRetryInterval"/> value must be greater than <see cref="TimeSpan.Zero"/>.
/// </exception>
public HttpRetryOptions(TimeSpan firstRetryInterval, int maxNumberOfAttempts, IList<HttpStatusCode>? statusCodesToRetry = null)
public HttpRetryOptions(IList<HttpStatusCode>? statusCodesToRetry = null)
{
this.MaxRetryInterval = DefaultMaxRetryinterval;

this.firstRetryInterval = firstRetryInterval;
this.maxNumberOfAttempts = maxNumberOfAttempts;
this.statusCodesToRetry = statusCodesToRetry ?? new List<HttpStatusCode>();
this.StatusCodesToRetry = statusCodesToRetry ?? new List<HttpStatusCode>();
}

/// <summary>
Expand All @@ -46,11 +33,7 @@ public HttpRetryOptions(TimeSpan firstRetryInterval, int maxNumberOfAttempts, IL
/// The TimeSpan to wait for the first retries.
/// </value>
[JsonPropertyName("FirstRetryInterval")]
public TimeSpan FirstRetryInterval
{
get { return this.firstRetryInterval; }
set { this.firstRetryInterval = value; }
}
public TimeSpan FirstRetryInterval { get; set; }

/// <summary>
/// Gets or sets the max retry interval.
Expand All @@ -59,7 +42,7 @@ public TimeSpan FirstRetryInterval
/// The TimeSpan of the max retry interval, defaults to 6 days.
/// </value>
[JsonPropertyName("MaxRetryInterval")]
public TimeSpan MaxRetryInterval { get; set; }
public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryinterval;

/// <summary>
/// Gets or sets the backoff coefficient.
Expand All @@ -68,7 +51,7 @@ public TimeSpan FirstRetryInterval
/// The backoff coefficient used to determine rate of increase of backoff. Defaults to 1.
/// </value>
[JsonPropertyName("BackoffCoefficient")]
public double BackoffCoefficient { get; set; }
public double BackoffCoefficient { get; set; } = 1;

/// <summary>
/// Gets or sets the timeout for retries.
Expand All @@ -77,7 +60,7 @@ public TimeSpan FirstRetryInterval
/// The TimeSpan timeout for retries, defaults to <see cref="TimeSpan.MaxValue"/>.
/// </value>
[JsonPropertyName("RetryTimeout")]
public TimeSpan RetryTimeout { get; set; }
public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue;

/// <summary>
/// Gets or sets the max number of attempts.
Expand All @@ -86,11 +69,7 @@ public TimeSpan FirstRetryInterval
/// The maximum number of retry attempts.
/// </value>
[JsonPropertyName("MaxNumberOfAttempts")]
public int MaxNumberOfAttempts
{
get { return this.maxNumberOfAttempts; }
set { this.maxNumberOfAttempts = value; }
}
public int MaxNumberOfAttempts { get; set; }

/// <summary>
/// Gets or sets the list of status codes upon which the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ public static class TaskOrchestrationContextExtensionMethods
/// <param name="context">The task orchestration context.</param>
/// <param name="request">The DurableHttpRequest used to make the HTTP call.</param>
/// <returns>DurableHttpResponse</returns>
public static async Task<DurableHttpResponse> CallHttpAsync(this TaskOrchestrationContext context, DurableHttpRequest request)
public static Task<DurableHttpResponse> CallHttpAsync(this TaskOrchestrationContext context, DurableHttpRequest request)
{
DurableHttpResponse response = await context.CallActivityAsync<DurableHttpResponse>(Constants.HttpTaskActivityReservedName, request);

return response;
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}

return context.CallActivityAsync<DurableHttpResponse>(Constants.HttpTaskActivityReservedName, request);
}

/// <summary>
Expand All @@ -36,16 +39,13 @@ public static async Task<DurableHttpResponse> CallHttpAsync(this TaskOrchestrati
/// <param name="content">Content passed in the HTTP request.</param>
/// <param name="retryOptions">The retry option for the HTTP task.</param>
/// <returns>A <see cref="Task{DurableHttpResponse}"/>Result of the HTTP call.</returns>
public static async Task<DurableHttpResponse?> CallHttpAsync(this TaskOrchestrationContext context, HttpMethod method, Uri uri, string? content = null, HttpRetryOptions? retryOptions = null)
public static Task<DurableHttpResponse> CallHttpAsync(this TaskOrchestrationContext context, HttpMethod method, Uri uri, string? content = null, HttpRetryOptions? retryOptions = null)
{
DurableHttpRequest request = new DurableHttpRequest(
method: method,
uri: uri,
content: content,
httpRetryOptions: retryOptions);
DurableHttpRequest request = new DurableHttpRequest(method, uri);

DurableHttpResponse response = await context.CallActivityAsync<DurableHttpResponse>(Constants.HttpTaskActivityReservedName, request);
request.Content = content;
request.HttpRetryOptions = retryOptions;

return response;
return context.CallHttpAsync(request);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,4 @@
<PackagePath>content/SBOM</PackagePath>
</Content>
</ItemGroup>
<ItemGroup>
<Folder Include="HTTP\" />
</ItemGroup>

</Project>

0 comments on commit d0e6e99

Please sign in to comment.