search

aliostad / CacheCow

An implementation of HTTP Caching in .NET Core and 4.5.2+ for both the client and the server
MIT License
849 stars 171 forks source link

MessageContentHttpMessageSerializer does not serialize HttpResponseMessage.RequestMessage #198

Closed xela30 closed 6 years ago

xela30 commented 6 years ago

Hi, Ali. Why not to serialize/deserialize HttpResponseMessage.RequestMessage? Sometimes consumers of response rely on it's reference to respective HttpRequestMessage.

aliostad commented 6 years ago

Hi,

The reason is the request that gets you the data is not the same as the one that resulted in the response be cached.

But IIRC, I set the request property back to your request and return the response. Do you see differently? If so, it would be a bug.

xela30 commented 6 years ago

Thanks for quick turnaround. It definitely makes sense. Indeed assigning subsequent request to cached response would be perfect solution but unfortunately I don't see it happens in my particular case. Perhaps I misused the lib. Can you point me to line of code where that assignment occurs?

aliostad commented 6 years ago

No worries, let me look into it and add a unit test for it which seems to be missing.

aliostad commented 6 years ago

Do you use 4.5 versions, e.g. CacheCow.Client45? It is highly recommended that you use those.

Regardless, I set the request of the response here:

https://github.com/aliostad/CacheCow/blob/dotnet45/src/CacheCow.Client/CachingHandler.cs#L291

aliostad commented 6 years ago

Can you please send a repro? As I said, I am setting it in the code so response.RequestMessage should not be null.

xela30 commented 6 years ago

It's weird but I can't reproduce it anymore. It seems there was something wrong on my side. BTW, what's the difference between Client and Client45 in nutshell? Is there Cachecow.Client.RedisCacheStore for 4.5?

aliostad commented 6 years ago

Parts completely re-written with async/await. On high load, the old one could get what seems to be a thread deadlock. Never seen again with x45 ones.

I am closing the issue, happy to open if you manage to repro.