Populating Cookies using requestData headers #642
Conversation
| @@ -1,8 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
Was this file causing a problem? If not, we should revert these changes to avoid increasing the scope of the PR
There was a problem hiding this comment.
I was making changes from this PR to allow local debugging - will revert when I no longer need to debug it.
| public static IReadOnlyCollection<IHttpCookie> CookieStringsToHttpCookie(KeyValuePair<string, IEnumerable<string>> cookieStrings) | ||
| { | ||
| // cookieStrings.Value comes in the format "Cookie_1=value;Cookie_2=value;Cookie_3=value" | ||
| var separateCookies = cookieStrings.Value.First().ToString().Split(";"); |
There was a problem hiding this comment.
With the changes proposed above, you should be able to modify this method to work directly with the string.
| }); | ||
| } | ||
|
|
||
| private IReadOnlyCollection<IHttpCookie> ToHttpCookies(IEnumerable<string> cookieString) |
There was a problem hiding this comment.
nit: arg name sounds singular while the type is a collection. May be you can change the type to string and pass the first item in line 41 ?
| if (cookieString != null && cookieString.Any()) | ||
| { | ||
|
|
||
| return ToHttpCookies(cookieString.First().ToString()); |
There was a problem hiding this comment.
Is the ToString() call really needed ? cookieString.First() is returning a string.
|
|
||
| public GrpcHttpRequestData(RpcHttp httpData, FunctionContext functionContext) | ||
| : base(functionContext) | ||
| { | ||
| _httpData = httpData ?? throw new ArgumentNullException(nameof(httpData)); | ||
| _cookies = new Lazy<IReadOnlyCollection<IHttpCookie>>(() => | ||
| { | ||
| if(Headers is null) |
There was a problem hiding this comment.
nit: Add a whitespace after if. (Visual studio has a"fix formatting" quick action item which will do these kind of things for you. Keep cursor here and press Ctrl + . )
| @@ -66,7 +102,13 @@ public override Stream Body | |||
|
|
|||
| public override HttpHeadersCollection Headers => _headers ??= new HttpHeadersCollection(_httpData.NullableHeaders.Select(h => new KeyValuePair<string, string>(h.Key, h.Value.Value))); | |||
There was a problem hiding this comment.
Unrelated question: Should we consider bringing the lazy loading behavior to Headers property as well ?
| }); | ||
| } | ||
|
|
||
| private IReadOnlyCollection<IHttpCookie> ToHttpCookies(string cookieString) |
There was a problem hiding this comment.
nit: Consider moving the private method down, after all public members.
First iteration of populating cookies in the HttpRequestData using the headers.