Support for response caching about proxykit

[jkonecki]

Just came up with the same myself ;-) I don't really mind about not using DI for cache store - the whole proxy thanks to your lib fits on one screen :-D

…

On Sat, 26 Jan 2019, 12:31 Damian Hickey ***@***.*** wrote: So this appears to work now: var cacheStore = new InMemoryCacheStore(TimeSpan.FromMinutes(1));services.AddProxy(options => { options.GetMessageHandler = () => new CachingHandler(cacheStore) { InnerHandler = new HttpClientHandler {AllowAutoRedirect = false, UseCookies = false} }; }); Not quite pit-of-success... 1. options.GetMessageHandler is hooked into the HttpClient Factory as the Primary handler. But here CachingHandler is a DelegatingHandler and I didn't put hook inplace for that. Hence manually setting the InnderHandler.. 2. We're not leveraging the IoC container to activate InMemoryCacheStore nor CachingHandler (with ICacheStore being injected into it). Need to think about this API a bit. Example CacheCow test code here: https://github.com/damianh/ProxyKit/blob/cachecow/src/ProxyKit.Tests/CacheCowTests.cs#L19 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABsWj9EZxpbdiizh_gZJjmZosEeCOWL2ks5vHEqZgaJpZM4aQ5DT> .

[jkonecki]

I think the recipe is a perfect solution - allows for full customisation which would require a lot of work if CacheCow was hidden behind ProxyKit façade.

…

On Sun, 27 Jan 2019, 12:38 Damian Hickey ***@***.*** wrote: Production already? Lovely! 😄 Based on this discussion, I made some improvements to make this easier: #37 <#37> . This change was semver breaking so new major version. Also added recipe for CacheCow and am supporting it via tests - https://github.com/damianh/ProxyKit/blob/master/src/Recipes/11_CachingWithCacheCow.cs - https://github.com/damianh/ProxyKit/blob/master/src/ProxyKit.Tests/CacheCowTests.cs I don't think an integration package is worth it right now but will keep open mind to it if there is more demand and makes things easier. Thanks for suggestion and the help! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABsWj25WYeVCGtvl9DU7KhdaSxHpthiNks5vHZ3ZgaJpZM4aQ5DT> .

[damianh]

Good idea! Wonder if I can leverage something existing to do the work i.e. CacheCow

[jkonecki]

I know Ali (the world is so small ;-) ) That would be cool - the only question is how to handle the dependency. Separate NuGet package maybe?

…

On Fri, 25 Jan 2019, 18:26 Damian Hickey ***@***.*** wrote: Good idea! Wonder if I can leverage something existing to do the work i.e. CacheCow — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABsWj1TirXTezoMANmKoQgWZWSFNLw4Hks5vG0xJgaJpZM4aQ5DT> .

[jkonecki]

I've done a spike and the caching can be simply implemented using:

var client = ClientExtensions.CreateClient();
app.RunProxy(context =>
{
    var forwardContext = context.ForwardTo("upstream-host-url");

    return client.SendAsync(forwardContext.UpstreamRequest);
});

Since CacheCow supports using HttpRequestMessage and HttpResponseMessage, it integrates nicely with Execute().

I would like to live it to you to decide on the best integration design.

[damianh]

Good start. The small problem with this is that is it not using the HttpClient factory.

[damianh]

Separate NuGet package maybe?

I think I'll go along with recipe right now as opposed to maintaining integration packages.

[jkonecki]

I assume, Damian, that you would like CacheCow to use the HttpClient instance injected into ForwardingContext instead of using own instance?

[jkonecki]

I've just noticed that ProxyMiddleware doesn't seem to be using HttpClientFactory provided in the constructor. Is this by design?

[damianh]

No, that's dead code that I forgot to remove (I'll remove it now). The code of interest is:

https://github.com/damianh/ProxyKit/blob/3b1f34d072b13e60ed639d1f7edf6b5c9c14160f/src/ProxyKit/ProxyContextExtensions.cs#L31-L33

[jkonecki]

Does it mean that if I register HttpClient with CacheCow caching handler than it should be picked up here and used by ProxyKit? Or am I missing something?

…

On Sat, 26 Jan 2019, 11:36 Damian Hickey ***@***.*** wrote: No, that's dead code that I forgot to remove (I'll remove it now). The code of interest is: https://github.com/damianh/ProxyKit/blob/3b1f34d072b13e60ed639d1f7edf6b5c9c14160f/src/ProxyKit/ProxyContextExtensions.cs#L31-L33 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABsWj5q8ACQXlzievYx_xG_Nb6Bq55wXks5vHD3ZgaJpZM4aQ5DT> .

[damianh]

SharedProxyOptions has a place to insert a custom HttpMessageHandler that is used when creating the HttpMessageHandler pipeline when creating the HttpClient that is injected into ProxyKitClient (aka "typed http client"). Am going to see if it's easy to inject CachingHandler here.

The way HttpClientFactory works is by disposing and re-creating all the handlers on a timed interval. I have a funny feeling that ProxyKit's API as it currently is might not quite be as simple as desired.

[jkonecki]

Hooking up a static instance of https://github.com/aliostad/CacheCow/blob/master/src/CacheCow.Client/CachingHandler.cs should do the job, shouldn't it?

…

On Sat, 26 Jan 2019, 12:00 Damian Hickey ***@***.*** wrote: SharedProxyOptions has a place to insert a custom HttpMessageHandler that is used when creating the HttpMessageHandler pipeline when creating the HttpClient that is injected into ProxyKitClient (aka "typed http client" <https://www.hanselman.com/blog/HttpClientFactoryForTypedHttpClientInstancesInASPNETCore21.aspx>). Am going to see if it's easy to inject CachingHandler here. The way HttpClientFactory works is by disposing and re-creating all the handlers on a timed interval. I have a funny feeling that ProxyKit's API as it currently is might not quite be as simple as desired. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABsWjy6PmZ2opSad2khZWe9HwvlwtHQRks5vHENxgaJpZM4aQ5DT> .

[damianh]

Can't be static. HttpClientFactory will want to dispose it and recreate it. This can result in ObjectDisposedExceptions a couple of minutes into a running system (which doesn't show up in tests). Related #8

[jkonecki]

Ok. There is an overload that takes a Cache store. I can create a single static instance of the store but keep recreating the handler itself.

…

On Sat, 26 Jan 2019, 12:12 Damian Hickey ***@***.*** wrote: Can't be static. HttpClientFactory will want to dispose it and recreate it. This can result in ObjectDisposedExceptions a couple of minutes into a running system (which doesn't show up in tests). Related #8 <#8> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABsWj0q2CDrgRZoxROGG6bWyPFAZnsrDks5vHEZLgaJpZM4aQ5DT> .

[damianh]

Yes, singleton CacheStore

[damianh]

So this appears to work now:

var cacheStore = new InMemoryCacheStore(TimeSpan.FromMinutes(1));
services.AddProxy(options =>
{
	options.GetMessageHandler = () => new CachingHandler(cacheStore)
	{
		InnerHandler = new HttpClientHandler {AllowAutoRedirect = false, UseCookies = false}
	};
});

Not quite pit-of-success...

1. options.GetMessageHandler is hooked into the HttpClient Factory as the Primary handler. But here CachingHandler is a DelegatingHandler and I didn't put hook inplace for such. Hence manually setting the InnderHandler...
2. We're not leveraging the IoC container to activate InMemoryCacheStore nor CachingHandler (with ICacheStore being injected into it).

Need to think about this API a bit.

Example CacheCow test code here: https://github.com/damianh/ProxyKit/blob/cachecow/src/ProxyKit.Tests/CacheCowTests.cs#L19

[jkonecki]

I think it may be safer to hold own instance of InMemoryVaryHeaderStore as well

…

On Sat, 26 Jan 2019, 12:34 Jakub Konecki ***@***.*** wrote: Just came up with the same myself ;-) I don't really mind about not using DI for cache store - the whole proxy thanks to your lib fits on one screen :-D On Sat, 26 Jan 2019, 12:31 Damian Hickey ***@***.*** wrote: > So this appears to work now: > > var cacheStore = new InMemoryCacheStore(TimeSpan.FromMinutes(1));services.AddProxy(options => > { > options.GetMessageHandler = () => new CachingHandler(cacheStore) > { > InnerHandler = new HttpClientHandler {AllowAutoRedirect = false, UseCookies = false} > }; > }); > > Not quite pit-of-success... > > 1. options.GetMessageHandler is hooked into the HttpClient Factory as > the Primary handler. But here CachingHandler is a DelegatingHandler > and I didn't put hook inplace for that. Hence manually setting the > InnderHandler.. > 2. We're not leveraging the IoC container to activate > InMemoryCacheStore nor CachingHandler (with ICacheStore being > injected into it). > > Need to think about this API a bit. > > Example CacheCow test code here: > https://github.com/damianh/ProxyKit/blob/cachecow/src/ProxyKit.Tests/CacheCowTests.cs#L19 > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#36 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABsWj9EZxpbdiizh_gZJjmZosEeCOWL2ks5vHEqZgaJpZM4aQ5DT> > . >

[damianh]

You do need to strongly manage the lifecycles of those, yes.

[jkonecki]

Thank you very much, Damian!
All working lovely in production :-)

[damianh]

Production already? Lovely! 😄

Based on this discussion, I made some improvements to make this easier: #37 . This change was semver breaking so new major version (on nuget.org soon).

Also added recipe for CacheCow and am supporting it via tests

• https://github.com/damianh/ProxyKit/blob/master/src/Recipes/11_CachingWithCacheCow.cs
• https://github.com/damianh/ProxyKit/blob/master/src/ProxyKit.Tests/CacheCowTests.cs

I don't think an integration package is worth it right now but will keep open mind to it if there is more demand and makes things easier.

Thanks for suggestion and the help!