redarrowlabs / Argo

:squirrel: c# object => json.api relational mapping
MIT License
7 stars 3 forks source link

Content is null in HttpResponseMessage given to ResponseHandler methods #68

Open zeitlerc opened 7 years ago

zeitlerc commented 7 years ago

Copying Content to the ResponseHandler methods is a TODO, so Content is always null. The HttpResponseMessage given to these methods is only a copy of the actual message instance.

.UseResponseHandler(new ResponseHandlerOptions{
        ResponseReceived = response => YourAsyncMethod(response),
        ResourceCreated = response => YourAsyncMethod(response),
        ResourceUpdated = response => YourAsyncMethod(response),
        ResourceRetrieved = response => YourAsyncMethod(response),
        ResourceDeleted = response => YourAsyncMethod(response),
      })

https://github.com/redarrowlabs/Argo/blob/9750289838b42ad007f858896993594e217d4f56/src/RedArrow.Argo.Client/Http/Handlers/Response/ResponseHandler.cs#L115

zeitlerc commented 7 years ago

The term Response Handler here is slightly misleading, since those methods cannot impact the response received by Argo. Response Listener would be more accurate, since those methods are only informed that a response happened. Returning a copy of an HttpResponseMessage is also misleading, since that copied object will likely not behave in the same way as the actual response. Since the Content was not copied over, I wasted time thinking that there might have been no content on the response. If the response content is never intended to be sent to these methods, I would recommend creating a new class containing the subset of copied response data. Reading the content here would be tricky. The task calling the response handler methods is "intentionally not awaited" (for performance reason I assume). If the same content stream is returned for both the model serialization and response handler methods, it's possible that they would read the stream at the same time, receiving garbage fragments of the full data. The easiest solution might be to async copy the response stream to a stream and a string, sending the stream for serialization and the string to the response handler methods.

zeitlerc commented 6 years ago

I got tricked again!!! This time, trying to publish events when certain data was updated. Since content on both request and response are null, there's no context as to which properties were updated in the ResourceUpdated handler. The only potential context would be the URL itself, which would need to be parsed for any useful info.