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

[fix] Handle serializing array/list parameters in GET/DELETE query strings #601

Merged
merged 3 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
23 changes: 22 additions & 1 deletion EasyPost.Tests/HttpTests/RequestTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.Net.Http;
using EasyPost._base;
using EasyPost.Http;
using Xunit;
Expand All @@ -13,9 +14,29 @@ public class RequestTest
[Fact]
public void TestRequestDisposal()
{
Request request = new("https://google.com", "not_a_real_endpoint", Method.Get, ApiVersion.V2, new Dictionary<string, object> { { "key", "value" } }, new Dictionary<string, string> { { "header_key", "header_value" } });
Request request = new("https://example.com", "not_a_real_endpoint", Method.Get, ApiVersion.V2, new Dictionary<string, object> { { "key", "value" } }, new Dictionary<string, string> { { "header_key", "header_value" } });
CustomAssertions.DoesNotThrow(() => request.Dispose());
}

[Fact]
public void TestQueryParameterList()
{
var parameters = new Dictionary<string, object>
{
{ "key1", new List<string> { "value1", "value2" } },
{ "key2", "value3" }
};

Request request = new("https://example.com", "not_a_real_endpoint", Method.Get, ApiVersion.V2, parameters, new Dictionary<string, string> { { "header_key", "header_value" } });

HttpRequestMessage httpRequestMessage = request.AsHttpRequestMessage();

string queryString = httpRequestMessage.RequestUri?.Query;

Assert.Contains("key1[]=value1", queryString); // Does not account for URL encoding
Assert.Contains("key1[]=value2", queryString); // Does not account for URL encoding
Assert.Contains("key2=value3", queryString); // Does not account for URL encoding
}

#endregion
}
48 changes: 39 additions & 9 deletions EasyPost/Http/Request.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Net.Http;
Expand Down Expand Up @@ -115,6 +116,9 @@ private void BuildQueryParameters()
// add query parameters
NameValueCollection query = HttpUtility.ParseQueryString(string.Empty);

// Separately store the list parameters to add them last
List<string> listParameters = new();

// build query string from parameters
foreach (KeyValuePair<string, object> param in _parameters)
{
Expand All @@ -124,26 +128,52 @@ private void BuildQueryParameters()
continue;
}

query[param.Key] = param.Value switch
var @switch = new SwitchCase
{
// TODO: Handle special conversions for other types
// DateTime dateTime => dateTime.ToString("o", CultureInfo.InvariantCulture),
var _ => param.Value.ToString(),
{ param.Value is IList, () => listParameters = AddListQueryParameter(listParameters, param.Key, (IList)param.Value) },
{ SwitchCaseScenario.Default, () => query[param.Key] = param.Value.ToString() },
};
@switch.MatchFirstTrue();
}

// short circuit if no query parameters
if (query.Count == 0)
// Finalize the query string
string queryString = query.ToString() ?? string.Empty;

// Add list parameters to the query string
string parameterCharacter = queryString.Length == 0 ? "?" : "&";
foreach (string pair in listParameters)
{
return;
queryString += $"{parameterCharacter}{pair}";
parameterCharacter = "&";
}

// rebuild the request URL with the query string appended
var uriBuilder = new UriBuilder(_requestMessage.RequestUri!)
{
Query = query.ToString(),
Query = queryString,
};
_requestMessage.RequestUri = new Uri(uriBuilder.ToString());

// _requestMessage.RequestUri = new Uri(uriBuilder.ToString());
_requestMessage.RequestUri = uriBuilder.Uri;
}

private static List<string> AddListQueryParameter(List<string> pairs, string key, IList value)
{
string keyPrefix = $"{HttpUtility.UrlEncode(key)}[]";
// ReSharper disable once LoopCanBeConvertedToQuery
foreach (object? item in value)
{
string? itemString = item?.ToString();
if (itemString == null)
{
continue;
}

string pair = $"{keyPrefix}={HttpUtility.UrlEncode(itemString)}";
pairs.Add(pair);
}

return pairs;
}

/// <inheritdoc cref="EasyPostClient._isDisposed"/>
Expand Down
1 change: 1 addition & 0 deletions EasyPost/Parameters/Tracker/All.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@
StartDatetime = dictionary.GetOrNull<string>("start_datetime"),
EndDatetime = dictionary.GetOrNull<string>("end_datetime"),
Carrier = dictionary.GetOrNull<string>("carrier"),
TrackingCode = dictionary.GetOrNull<string>("tracking_code"),

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / Coverage_Requirements

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / lint

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / NET_Tests (Net50)

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / NET_Tests (Net60)

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / Visual_Basic_Compatibility_Test

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / FSharp_Compatibility_Tests

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / NET_Tests (Net70)

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / NET_Tests (NetStandard20)

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'

Check warning on line 86 in EasyPost/Parameters/Tracker/All.cs

View workflow job for this annotation

GitHub Actions / NET_Tests (Net80)

'All.TrackingCode' is obsolete: 'This property will be removed in a future version and replaced with TrackingCodes.'
TrackingCodes = dictionary.GetOrNull<List<string>>("tracking_codes"),
};
}
}
Expand Down
Loading