Formatting of Extra Property #218
Conversation
to address git hub issues #200 #201 implementation to properly format the Extra Property of the LiveSchema and PrerecordedSchema for the QueryString of request. The implementation in the QueryParameterUtil class, in the URl Encode methods switch statement. the format does Not change the casing of the key's or value's as the casing may have significance to the user
WalkthroughThe recent update introduces a significant enhancement to the handling of dictionary parameters in query strings. Specifically, it focuses on improving the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| //specific case for the Extra Parameter dictionary to format the querystring correctly | ||
| //no case changing of the key or values as theses are unknowns and the casing may have | ||
| //significance to the user | ||
| case Dictionary<string, string> dict: | ||
| if (name == "extra") | ||
| { | ||
| foreach (var kvp in dict) | ||
| { | ||
| sb.Append($"{name}={HttpUtility.UrlEncode($"{kvp.Key}:{kvp.Value}")}&"); | ||
| } | ||
| } | ||
| break; |
There was a problem hiding this comment.
The implementation for handling Dictionary<string, string> in the UrlEncode method correctly preserves the case of keys and values, aligning with the PR objectives. However, consider using Uri.EscapeDataString instead of HttpUtility.UrlEncode for encoding keys and values. Uri.EscapeDataString provides a more RFC 3986 compliant way to encode URI components, which might be more appropriate for query string parameters.
- sb.Append($"{name}={HttpUtility.UrlEncode($"{kvp.Key}:{kvp.Value}")}&");
+ sb.Append($"{name}={Uri.EscapeDataString($"{kvp.Key}:{kvp.Value}")}&");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| //specific case for the Extra Parameter dictionary to format the querystring correctly | |
| //no case changing of the key or values as theses are unknowns and the casing may have | |
| //significance to the user | |
| case Dictionary<string, string> dict: | |
| if (name == "extra") | |
| { | |
| foreach (var kvp in dict) | |
| { | |
| sb.Append($"{name}={HttpUtility.UrlEncode($"{kvp.Key}:{kvp.Value}")}&"); | |
| } | |
| } | |
| break; | |
| //specific case for the Extra Parameter dictionary to format the querystring correctly | |
| //no case changing of the key or values as theses are unknowns and the casing may have | |
| //significance to the user | |
| case Dictionary<string, string> dict: | |
| if (name == "extra") | |
| { | |
| foreach (var kvp in dict) | |
| { | |
| sb.Append($"{name}={Uri.EscapeDataString($"{kvp.Key}:{kvp.Value}")}&"); | |
| } | |
| } | |
| break; |
| public void GetParameters_Should_Return_String_When_Passing_Dictonary_Parameter() | ||
| { | ||
| //Arrange | ||
| var prerecordedOptions = new PrerecordedSchema() | ||
| { | ||
| Extra = new Dictionary<string, string> | ||
| { | ||
| {"KeyOne","ValueOne" }, | ||
| {"KeyTwo","ValueTwo" } | ||
| } | ||
| }; | ||
| var expected = $"extra={HttpUtility.UrlEncode("KeyOne:ValueOne")}&extra={HttpUtility.UrlEncode("KeyTwo:ValueTwo")}"; | ||
|
|
||
| //Act | ||
| var result = QueryParameterUtil.GetParameters(prerecordedOptions); | ||
|
|
||
| //Assert | ||
| result.Should().NotBeNull(); | ||
| result.Should().Contain(expected); | ||
| } |
There was a problem hiding this comment.
The test GetParameters_Should_Return_String_When_Passing_Dictonary_Parameter correctly asserts the functionality for handling dictionary parameters. However, the expected string construction does not account for the order in which dictionary entries might be processed, which could lead to flaky tests if the dictionary iteration order is not consistent. Consider using a more flexible assertion that checks for the presence of each key-value pair in the result, regardless of order.
- var expected = $"extra={HttpUtility.UrlEncode("KeyOne:ValueOne")}&extra={HttpUtility.UrlEncode("KeyTwo:ValueTwo")}";
+ // Assert each key-value pair separately to avoid dependency on dictionary iteration order
+ Assert.IsTrue(result.Contains(HttpUtility.UrlEncode("KeyOne:ValueOne")));
+ Assert.IsTrue(result.Contains(HttpUtility.UrlEncode("KeyTwo:ValueTwo")));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| public void GetParameters_Should_Return_String_When_Passing_Dictonary_Parameter() | |
| { | |
| //Arrange | |
| var prerecordedOptions = new PrerecordedSchema() | |
| { | |
| Extra = new Dictionary<string, string> | |
| { | |
| {"KeyOne","ValueOne" }, | |
| {"KeyTwo","ValueTwo" } | |
| } | |
| }; | |
| var expected = $"extra={HttpUtility.UrlEncode("KeyOne:ValueOne")}&extra={HttpUtility.UrlEncode("KeyTwo:ValueTwo")}"; | |
| //Act | |
| var result = QueryParameterUtil.GetParameters(prerecordedOptions); | |
| //Assert | |
| result.Should().NotBeNull(); | |
| result.Should().Contain(expected); | |
| } | |
| public void GetParameters_Should_Return_String_When_Passing_Dictonary_Parameter() | |
| { | |
| //Arrange | |
| var prerecordedOptions = new PrerecordedSchema() | |
| { | |
| Extra = new Dictionary<string, string> | |
| { | |
| {"KeyOne","ValueOne" }, | |
| {"KeyTwo","ValueTwo" } | |
| } | |
| }; | |
| // Assert each key-value pair separately to avoid dependency on dictionary iteration order | |
| Assert.IsTrue(result.Contains(HttpUtility.UrlEncode("KeyOne:ValueOne"))); | |
| Assert.IsTrue(result.Contains(HttpUtility.UrlEncode("KeyTwo:ValueTwo"))); | |
| //Act | |
| var result = QueryParameterUtil.GetParameters(prerecordedOptions); | |
| //Assert | |
| result.Should().NotBeNull(); | |
| result.Should().Contain(expected); | |
| } |
to address git hub issues
#200
#201
implementation to properly format the Extra Property of the LiveSchema and PrerecordedSchema for the QueryString of request. The implementation in the QueryParameterUtil class, in the URL Encode methods switch statement. the format does NOT change the casing of the keys or value's as the casing may have significance to the user
Summary by CodeRabbit