For discussion: Replatform FhirClient on HttpClient #727
Conversation
| string LastBodyAsText { get; } | ||
| HttpWebRequest LastRequest { get; } | ||
| HttpWebResponse LastResponse { get; } | ||
| Bundle.ResponseComponent LastResult { get; } |
There was a problem hiding this comment.
breaking change
| } | ||
|
|
||
| public HttpWebResponse RawResponse { get; internal set; } | ||
| public byte[] Body { get; internal set; } | ||
| public HttpResponseMessage Response { get; internal set; } |
There was a problem hiding this comment.
breaking change
|
I haven't had a good look over these tests, but one if the things I'd like to see with the client is a pure streaming option that doesn't read the lot into a string first... but that has issues with the existing event handler approach. |
|
This doesn't read the body for the event handlers, it could be a stream. But the first step of parsing reads it into a string because that is what the parsers accept, I didn't change that. It should be possible to read the the content as a stream, create an JsonTextReader/XmlTextReader and pass that to the parser. I can give it a try |
|
There is a streaming version if the parsers too, in fact I believe the string version uses a string stream... |
|
The old code reads it so that the event handler doesn't mess with the stream as it can't rewind... |
|
@brianpos I was playing around and I think it should be possible to serialize through a stream directly into the network request. That might reduce allocations a bit, I think one less copy of the xml/json bytes in memory. Did you have a particular use case in mind for a streaming API? |
I don't have much confidence in these changes yet. I can't get the Integration tests to pass with any of the listed DSTU2 endpoints, even before any changes.
The extent of the public API changes are:
Last...properties fromIFhirClient, I felt these were problematic because they makeFhirClientnot threadsafe. For test code, it was trivial to write a custom httpclienthandler that captures these.HttpRequest/ResponseMessageMost of the code was back ported from #507 that was merged into develop-r4.
Also addresses #478
Would such API changes be acceptable? Or must an HttpClient implementation remain separate?