Skip to content

Commit

Permalink
[chore] Cancellation token best practice, parameter cleanup (#457)
Browse files Browse the repository at this point in the history
- Move explicit parameters (e.g. report type on All function) into parameters object, so all signatures with parameter objects are one or two length only
- Fix incorrect nullability of ID for some API calls
- Remove Parameter functions calling Dictionary functions (Parameter functions are now self-contained, easier to migrate and maintain)
- Add cancellation token option to all functions to allow end-user to cancel an HTTP call.
  • Loading branch information
nwithan8 committed May 2, 2023
1 parent 022ebcb commit ecb7370
Show file tree
Hide file tree
Showing 35 changed files with 456 additions and 313 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Collections.Generic;
using System.Threading.Tasks;
using EasyPost.Exceptions.General;
using EasyPost.Models.API;
using EasyPost.Tests._Utilities;
using EasyPost.Tests._Utilities.Attributes;
Expand Down Expand Up @@ -29,11 +30,12 @@ public async Task TestCreate()
{
{ "start_date", Fixtures.ReportDate },
{ "end_date", Fixtures.ReportDate },
{ "type", Fixtures.ReportType },
};

BetaFeatures.Parameters.Reports.Create parameters = Fixtures.Parameters.Reports.Create(data);

Report report = await Client.Report.Create(Fixtures.ReportType, parameters);
Report report = await Client.Report.Create(parameters);

Assert.IsType<Report>(report);
Assert.StartsWith(Fixtures.ReportIdPrefix, report.Id);
Expand All @@ -57,11 +59,12 @@ public async Task TestCreateWithAdditionalColumns()
{ "additional_columns", additionalColumns },
{ "start_date", Fixtures.ReportDate },
{ "end_date", Fixtures.ReportDate },
{ "type", "shipment" },
};

BetaFeatures.Parameters.Reports.Create parameters = Fixtures.Parameters.Reports.Create(data);

Report report = await Client.Report.Create("shipment", parameters);
Report report = await Client.Report.Create(parameters);

// verify parameters by checking VCR cassette for correct URL
// Some reports take a long time to generate, so we won't be able to consistently pull the report
Expand All @@ -85,11 +88,12 @@ public async Task TestCreateWithColumns()
{ "columns", columns },
{ "start_date", Fixtures.ReportDate },
{ "end_date", Fixtures.ReportDate },
{ "type", "shipment" },
};

BetaFeatures.Parameters.Reports.Create parameters = Fixtures.Parameters.Reports.Create(data);

Report report = await Client.Report.Create("shipment", parameters);
Report report = await Client.Report.Create(parameters);

// verify parameters by checking VCR cassette for correct URL
// Some reports take a long time to generate, so we won't be able to consistently pull the report
Expand All @@ -106,11 +110,11 @@ public async Task TestAll()
{
UseVCR("all");

Dictionary<string, object> data = new Dictionary<string, object>() { { "page_size", Fixtures.PageSize } };
Dictionary<string, object> data = new Dictionary<string, object>() { { "page_size", Fixtures.PageSize }, { "type", "shipment" } };

BetaFeatures.Parameters.Reports.All parameters = Fixtures.Parameters.Reports.All(data);

ReportCollection reportCollection = await Client.Report.All("shipment", parameters);
ReportCollection reportCollection = await Client.Report.All(parameters);

List<Report> reports = reportCollection.Reports;

Expand All @@ -133,13 +137,27 @@ public async Task TestAllParameterHandOff()

const string type = "shipment";

Dictionary<string, object> data = new Dictionary<string, object>() { { "page_size", Fixtures.PageSize } };
Dictionary<string, object> data = new Dictionary<string, object>() { { "page_size", Fixtures.PageSize }, { "type", type } };

BetaFeatures.Parameters.Reports.All parameters = Fixtures.Parameters.Reports.All(data);

ReportCollection reportCollection = await Client.Report.All(type, parameters);
ReportCollection reportCollection = await Client.Report.All(parameters);

Assert.Equal(type, ((BetaFeatures.Parameters.Reports.All)reportCollection.Filters!).ReportType);
Assert.Equal(type, ((BetaFeatures.Parameters.Reports.All)reportCollection.Filters!).Type);
}

[Fact]
[CrudOperations.Read]
[Testing.Exception]
public async Task TestMissingTypeWhenCallingAll()
{
UseVCR("missing_type_when_calling_all");

Dictionary<string, object> data = new Dictionary<string, object>() { { "page_size", Fixtures.PageSize } }; // missing type

BetaFeatures.Parameters.Reports.All parameters = Fixtures.Parameters.Reports.All(data);

await Assert.ThrowsAsync<MissingParameterError>(() => Client.Report.All(parameters));
}

#endregion
Expand Down
2 changes: 2 additions & 0 deletions EasyPost.Tests/Fixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ internal static BetaFeatures.Parameters.Reports.Create Create(Dictionary<string,
StartDate = fixture.GetOrNull<string>("start_date"),
IncludeChildren = fixture.GetOrNullBoolean("include_children"),
SendEmail = fixture.GetOrNullBoolean("send_email"),
Type = fixture.GetOrNull<string>("type"),
};
}

Expand All @@ -564,6 +565,7 @@ internal static BetaFeatures.Parameters.Reports.All All(Dictionary<string, objec
AfterId = fixture.GetOrNull<string>("after_id"),
StartDatetime = fixture.GetOrNull<string>("start_datetime"),
EndDatetime = fixture.GetOrNull<string>("end_datetime"),
Type = fixture.GetOrNull<string>("type"),
};
}
}
Expand Down
2 changes: 1 addition & 1 deletion EasyPost.Tests/ServicesTests/BatchServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public async Task TestGenerateScanForm()
Thread.Sleep(10000); // Wait enough time to process
}

batch = await Client.Batch.GenerateScanForm(batch.Id);
batch = await Client.Batch.GenerateScanForm(batch.Id, "pdf");

// We can't assert anything meaningful here because the scanform gets queued for generation and may not be immediately available
Assert.IsType<Batch>(batch);
Expand Down
2 changes: 1 addition & 1 deletion EasyPost.Tests/ServicesTests/ReportServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public async Task TestAllParameterHandOff()

ReportCollection reportCollection = await Client.Report.All(type, filters);

Assert.Equal(type, ((BetaFeatures.Parameters.Reports.All)reportCollection.Filters!).ReportType);
Assert.Equal(type, ((BetaFeatures.Parameters.Reports.All)reportCollection.Filters!).Type);
}

[Fact]
Expand Down
3 changes: 2 additions & 1 deletion EasyPost.Tests/TestUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Net.Http;
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
using EasyPost._base;
using EasyPost.Http;
Expand Down Expand Up @@ -220,7 +221,7 @@ internal sealed class MockClient : Client
private readonly List<MockRequest> _mockRequests = new();

#pragma warning disable CS1998
internal override async Task<HttpResponseMessage> ExecuteRequest(HttpRequestMessage request)
internal override async Task<HttpResponseMessage> ExecuteRequest(HttpRequestMessage request, CancellationToken cancellationToken)
#pragma warning restore CS1998
{
MockRequest? mockRequest = FindMatchingMockRequest(request);
Expand Down
13 changes: 7 additions & 6 deletions EasyPost/BetaFeatures/Parameters/Reports/All.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ public sealed class All : BaseAllParameters
{
#region Request Parameters

/// <summary>
/// What type of report to return. Required.
/// </summary>
// This parameter is not included in the request body, but is used to determine the endpoint to call.
public string? Type { get; set; }

/// <summary>
/// Only records created after the given ID will be included. May not be used with <see cref="BeforeId"/>.
/// </summary>
Expand Down Expand Up @@ -43,12 +49,6 @@ public sealed class All : BaseAllParameters
[TopLevelRequestParameter(Necessity.Optional, "start_datetime")]
public string? StartDatetime { get; set; }

/// <summary>
/// The type of reports to retrieve.
/// </summary>
// This is not set by the end-user, but stored for reference when getting the next page of results.
internal string? ReportType { get; set; }

#endregion

protected override TParameters FromDictionaryProtected<TParameters>(Dictionary<string, object> dictionary)
Expand All @@ -60,6 +60,7 @@ protected override TParameters FromDictionaryProtected<TParameters>(Dictionary<s
AfterId = dictionary.GetOrNull<string>("after_id"),
StartDatetime = dictionary.GetOrNull<string>("start_datetime"),
EndDatetime = dictionary.GetOrNull<string>("end_datetime"),
Type = dictionary.GetOrNull<string>("type"),
};

return (parameters as TParameters)!;
Expand Down
6 changes: 6 additions & 0 deletions EasyPost/BetaFeatures/Parameters/Reports/Create.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ public sealed class Create : BaseParameters, IReportParameter
{
#region Request Parameters

/// <summary>
/// What type of report to create. Required.
/// </summary>
// This parameter is not included in the request body, but is used to determine the endpoint to call.
public string? Type { get; set; }

[TopLevelRequestParameter(Necessity.Optional, "report", "additional_columns")]
public List<string>? AdditionalColumns { get; set; }

Expand Down
34 changes: 20 additions & 14 deletions EasyPost/Services/AddressService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using EasyPost._base;
using EasyPost.BetaFeatures.Parameters;
Expand Down Expand Up @@ -41,7 +42,7 @@ internal AddressService(EasyPostClient client)
/// </param>
/// <returns>EasyPost.Address instance.</returns>
[CrudOperations.Create]
public async Task<Address> Create(Dictionary<string, object> parameters)
public async Task<Address> Create(Dictionary<string, object> parameters, CancellationToken cancellationToken = default)
{
// Check verify and verify_strict presence in parameters
bool verify = parameters.ContainsKey("verify");
Expand All @@ -64,7 +65,7 @@ public async Task<Address> Create(Dictionary<string, object> parameters)
parameters.Add("verify_strict", true);
}

return await RequestAsync<Address>(Method.Post, "addresses", parameters);
return await RequestAsync<Address>(Method.Post, "addresses", cancellationToken, parameters);
}

/// <summary>
Expand All @@ -73,10 +74,10 @@ public async Task<Address> Create(Dictionary<string, object> parameters)
/// <param name="parameters"><see cref="BetaFeatures.Parameters.Addresses.Create"/> parameter set.</param>
/// <returns><see cref="Address"/> instance.</returns>
[CrudOperations.Create]
public async Task<Address> Create(BetaFeatures.Parameters.Addresses.Create parameters)
public async Task<Address> Create(BetaFeatures.Parameters.Addresses.Create parameters, CancellationToken cancellationToken = default)
{
// Because the normal Create method does wrapping internally, we can't simply pass the parameters object to it, otherwise it will wrap the parameters twice.
return await RequestAsync<Address>(Method.Post, "addresses", parameters.ToDictionary());
return await RequestAsync<Address>(Method.Post, "addresses", cancellationToken, parameters.ToDictionary());
}

/// <summary>
Expand All @@ -98,18 +99,18 @@ public async Task<Address> Create(BetaFeatures.Parameters.Addresses.Create param
/// </param>
/// <returns>An Address object.</returns>
[CrudOperations.Create]
public async Task<Address> CreateAndVerify(Dictionary<string, object> parameters) => await RequestAsync<Address>(Method.Post, "addresses/create_and_verify", parameters, "address");
public async Task<Address> CreateAndVerify(Dictionary<string, object> parameters, CancellationToken cancellationToken = default) => await RequestAsync<Address>(Method.Post, "addresses/create_and_verify", cancellationToken, parameters, "address");

/// <summary>
/// Create and verify an <see cref="Address"/>.
/// </summary>
/// <param name="parameters"><see cref="BetaFeatures.Parameters.Addresses.Create"/> parameter set.</param>
/// <returns><see cref="Address"/> instance.</returns>
[CrudOperations.Create]
public async Task<Address> CreateAndVerify(BetaFeatures.Parameters.Addresses.Create parameters)
public async Task<Address> CreateAndVerify(BetaFeatures.Parameters.Addresses.Create parameters, CancellationToken cancellationToken = default)
{
// Because the normal Create method does wrapping internally, we can't simply pass the parameters object to it, otherwise it will wrap the parameters twice.
return await RequestAsync<Address>(Method.Post, "addresses/create_and_verify", parameters.ToDictionary(), "address");
return await RequestAsync<Address>(Method.Post, "addresses/create_and_verify", cancellationToken, parameters.ToDictionary(), "address");
}

/// <summary>
Expand All @@ -128,9 +129,9 @@ public async Task<Address> CreateAndVerify(BetaFeatures.Parameters.Addresses.Cre
/// </param>
/// <returns>An EasyPost.AddressCollection instance.</returns>
[CrudOperations.Read]
public async Task<AddressCollection> All(Dictionary<string, object>? parameters = null)
public async Task<AddressCollection> All(Dictionary<string, object>? parameters = null, CancellationToken cancellationToken = default)
{
AddressCollection collection = await RequestAsync<AddressCollection>(Method.Get, "addresses", parameters);
AddressCollection collection = await RequestAsync<AddressCollection>(Method.Get, "addresses", cancellationToken, parameters);
collection.Filters = BaseAllParameters.FromDictionary<BetaFeatures.Parameters.Addresses.All>(parameters);
return collection;
}
Expand All @@ -141,7 +142,12 @@ public async Task<AddressCollection> All(Dictionary<string, object>? parameters
/// <param name="parameters"><see cref="BetaFeatures.Parameters.Addresses.All"/> parameter set.</param>
/// <returns><see cref="AddressCollection"/> instance.</returns>
[CrudOperations.Read]
public async Task<AddressCollection> All(BetaFeatures.Parameters.Addresses.All parameters) => await All(parameters.ToDictionary());
public async Task<AddressCollection> All(BetaFeatures.Parameters.Addresses.All parameters, CancellationToken cancellationToken = default)
{
AddressCollection collection = await RequestAsync<AddressCollection>(Method.Get, "addresses", cancellationToken, parameters.ToDictionary());
collection.Filters = parameters;
return collection;
}

/// <summary>
/// Get the next page of a paginated <see cref="AddressCollection"/>.
Expand All @@ -151,25 +157,25 @@ public async Task<AddressCollection> All(Dictionary<string, object>? parameters
/// <returns>The next page, as a <see cref="AddressCollection"/> instance.</returns>
/// <exception cref="EndOfPaginationError">Thrown if there is no next page to retrieve.</exception>
[CrudOperations.Read]
public async Task<AddressCollection> GetNextPage(AddressCollection collection, int? pageSize = null) => await collection.GetNextPage<AddressCollection, BetaFeatures.Parameters.Addresses.All>(async parameters => await All(parameters), collection.Addresses, pageSize);
public async Task<AddressCollection> GetNextPage(AddressCollection collection, int? pageSize = null, CancellationToken cancellationToken = default) => await collection.GetNextPage<AddressCollection, BetaFeatures.Parameters.Addresses.All>(async parameters => await All(parameters, cancellationToken), collection.Addresses, pageSize);

/// <summary>
/// Retrieve an Address from its id.
/// </summary>
/// <param name="id">String representing an Address. Starts with "adr_".</param>
/// <returns>EasyPost.Address instance.</returns>
[CrudOperations.Read]
public async Task<Address> Retrieve(string id) => await RequestAsync<Address>(Method.Get, $"addresses/{id}");
public async Task<Address> Retrieve(string id, CancellationToken cancellationToken = default) => await RequestAsync<Address>(Method.Get, $"addresses/{id}", cancellationToken);

/// <summary>
/// Verify an Address.
/// </summary>
/// <param name="id">ID of the address to verify.</param>
/// <returns>EasyPost.Address instance. Check message for verification failures.</returns>
[CrudOperations.Update]
public async Task<Address> Verify(string id)
public async Task<Address> Verify(string id, CancellationToken cancellationToken = default)
{
return await RequestAsync<Address>(Method.Get, $"addresses/{id}/verify", null, "address");
return await RequestAsync<Address>(Method.Get, $"addresses/{id}/verify", cancellationToken, rootElement: "address");
}

#endregion
Expand Down
3 changes: 2 additions & 1 deletion EasyPost/Services/ApiKeyService.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Threading;
using System.Threading.Tasks;
using EasyPost._base;
using EasyPost.Http;
Expand All @@ -21,7 +22,7 @@ internal ApiKeyService(EasyPostClient client)
/// </summary>
/// <returns>An EasyPost.ApiKeyCollection instances.</returns>
[CrudOperations.Read]
public async Task<ApiKeyCollection> All() => await RequestAsync<ApiKeyCollection>(Method.Get, "api_keys");
public async Task<ApiKeyCollection> All(CancellationToken cancellationToken = default) => await RequestAsync<ApiKeyCollection>(Method.Get, "api_keys", cancellationToken);

#endregion
}
Expand Down
Loading

0 comments on commit ecb7370

Please sign in to comment.