Add response headers to the serialized response #260
Conversation
|
I love the elegence of this implimentation. It might however be a good idea to include an example of using both |
docs/headers.md
Outdated
|
|
||
| Custom headers can be added by creating a new header option collection and adding it to the request object: | ||
|
|
||
| ```chsarp |
There was a problem hiding this comment.
Language tag typo this one and below
docs/headers.md
Outdated
| Custom headers can be added by creating a new header option collection and adding it to the request object: | ||
|
|
||
| ```chsarp | ||
| List<HeaderOption> options = new List<HeaderOption>(); |
There was a problem hiding this comment.
Above
"Custom headers can be added by creating a new header option collection"
change to:
"Custom headers can be added by creating a new option collection"
Below
Let's change this to List<Option> options = new List<Option>(); so that we can add QueryOption to the collection.
| if (response.Content != null) | ||
| { | ||
| var responseString = await response.Content.ReadAsStringAsync().ConfigureAwait(false); | ||
| responseString = AddHeadersToResponse(response, responseString); |
There was a problem hiding this comment.
Instead of passing the responseString, and the httpResponseMessage (which has the response content as well) to AddHeadersToResponse ,I suggest that we change this call from AddHeadersToResponse to string GetResponseString(HttpResponseMessage hrm). GetResponseString will call AddHeadersToResponse with the results of response.Content.ReadAsStringAsync() and then return the results it receives from AddHeadersToResponse. The goal being that we don't pass the same data twice to the function.
There was a problem hiding this comment.
That makes a lot more sense
docs/headers.md
Outdated
| ``` | ||
|
|
||
|
|
||
| *Currently, requests that have a return type of `void` do not return response headers and cannot be inspected.* |
There was a problem hiding this comment.
Stream return types won't work either. We should add an item to our breaking changes backlog for this.
MIchaelMainer
left a comment
There was a problem hiding this comment.
I don't think I did the review correctly as GH is saying I have 1 thing to review.
| var responseItem = await baseRequest.SendAsync<DerivedTypeClass>("string", CancellationToken.None); | ||
| Assert.IsNotNull(responseItem.AdditionalData["responseHeaders"], "No response headers available"); | ||
| Assert.AreEqual(expectedResponseItem.AdditionalData["responseHeaders"], responseItem.AdditionalData["responseHeaders"], "Unexpected response headers"); | ||
| } |
There was a problem hiding this comment.
Looks good as far as I see. Can you walk me through this so that I can understand how the mock library works.
This allows them to get deserialized down the line, typically by the default
Serializerclass intoAdditionalData["responseHeaders"]andAdditionalData["statusCode".BaseRequest.cs - add headers to the response content. If the content stream is null, do not add headers, because the serializer is expecting a null object back. Because these will get picked up by the individual models later in the deserialization process, we need to temporarily re-serialize the headers and append them to the content stream to "trick" the serializer into deserializing them back into the object as additional data.
MockSerializer - Added SerializeObject method to MockSerializer for both .NET and .NET Core tests (required for MockBehavior.Strict, also used in the BaseRequestTest.cs)
BaseRequestTests - Similar behavior to
SendAsynctest, but with content and headers set. Content must be set in order for headers to be attached.OneDriveTests and PlannerTests - Updated test credentials to verify functional tests pass with response header updates.