xyncro / freya

Freya Web Stack - Meta-Package
https://freya.io
Other
329 stars 30 forks source link

[Guidance needed] Freya and caching #175

Closed Vidarls closed 8 years ago

Vidarls commented 8 years ago

Currently working on getting my api to play nicely with caching clients, and I struggle a bit.

Cache-control

I can't find any trace of cache control directives using FreyaMachine.

When handling an authorized request, at least cache-control: private header should be present.

Also for a close-to-metal stack like Freya it would seems natural to provide a lot more control here, (of course with sensible default behaviour)

Expiry

Freya only seems to support the expired timestamp method, none of the more modern lifetime / shared lifetime, as cache-control does not seem to be implemented.

Vidarls commented 8 years ago

Note: I am very much not an expert of the http protocol, so I really want Freya to push me into the pit-of-success (:tm:)

kolektiv commented 8 years ago

You're right, machine at the moment doesn't really help much with cache control. It could, now that I think about it (at least in terms of helping with the response handler, taking that away from the end user). I'm not sure there's time to get that in to 3.0, although I'll take a look. If not, it'll definitely be in 4.0.

The same, I think, applies to the other cache control items - I think I need to have a session with the RFCs and work out whether they make sense as part of the processing flow. It may well be that it's something Freya can bring to the party which is quite new.

Your wish in terms of pit-of-success is absolutely reasonable, and very much what I want with Freya - these are both really good points, and I'm going to make sure Freya does as much as possible to help, whether in the very soon 3.0 release, or the "not quite as soon, but definitely not too long" 4.0 release.

kolektiv commented 8 years ago

One dependency needed is fuller support for those headers in Arachne as well, so I'll make sure that ends up on the slate. At the moment, you could probably make sure some of those headers are set directly from the handler in machine, but it isn't as nice as it could/should be.

Vidarls commented 8 years ago

I can set the headers directly in the handler, and the machine would respect those?

Vidarls commented 8 years ago

Can you give a pointer to how to do this with lenses?

kolektiv commented 8 years ago

It shouldn't touch them from that point onward. The machine isn't going to use them in any way as it stands, it just lets you take as much control in the handler as you'd like. So although it isn't as helpful as I'd like, it should "keep out of the way"!

kolektiv commented 8 years ago

I'll try and put a quick example together shortly :smile:

kolektiv commented 8 years ago

I think - and this is effectively from memory/looking at code - that cache control would look something like this (this assumes Freya 2.x)

let handler =
    freya {
        do! Freya.Lens.setPartial Response.Headers.CacheControl_ (CacheControl [ NoStore; Private ])

        ... } 

That uses the CacheControl type from Arachne (https://github.com/freya-fs/arachne/blob/master/src/Arachne.Http/Http.fs#L1626) with the lens to the appropriate response header. If you were to add that to your handlers (and add more directives to cache control if relevant) then the responses should include the appropriate header. I haven't tried it though, so let me know if it doesn't!

For reference also - in 3.x+ the same would look like this:

let handler =
    freya {
        do! Freya.Optic.set Response.Headers.cacheControl_ (Some (CacheControl [ NoStore; Private ]))

        ... } 
Vidarls commented 8 years ago

while waiting I did this to try to put all caching concerns in the same place:

(note: I want to enable client side caching, as performance is a concern, but I do not want intermediary caching as security is also a concern. (And yes, all is 100% https))

let setPrivateCache maxAge sMaxAge eTag = 
        freya {
            do! (Freya.Lens.setPartial Freya.Lenses.Http.Lenses.Response.Headers.CacheControl_ 
                (CacheControl(
                    [CacheDirective.Private; 
                     CacheDirective.SMaxAge(System.TimeSpan.FromSeconds(sMaxAge));
                     CacheDirective.MaxAge(System.TimeSpan.FromSeconds(maxAge))])))
            do! (Freya.Lens.setPartial Freya.Lenses.Http.Lenses.Response.Headers.Expires_ 
                (Expires(System.DateTime.UtcNow.AddSeconds(maxAge))))

            do! (Freya.Lens.setPartial Freya.Lenses.Http.Lenses.Response.Headers.ETag_ 
                (ETag(Weak(eTag))))
        }

Looks sensible?

edit: Replaced |> ignore with do!

kolektiv commented 8 years ago

Yes that should probably do it. If you open Freya.Lenses.Http and Arachne.Http then you should be able to shorten a lot of those long type references as well. Response.Headers.CacheControl_ is what I'd normally expect, etc.

Those Freya.Lens.setPartial calls should also use do! notation, so do! Freya.Lens.setPartial ... etc. with no need for |> ignore.

Hope that's helpful!

Vidarls commented 8 years ago

Will setting expired and ETag out of band like this disturb how the machine responds to requests?

Vidarls commented 8 years ago

:( Yes it will, I get 304 not modified always when e-tag is not set in machine, and I can not set e-tag in machine due to #176.

Is the prerelease of v3 somewhat stable?

kolektiv commented 8 years ago

No it shouldn't, the only change when not telling the machine about etags and last modified is that the machine won't do any precondition checking (so it won't be able to handle the conditional request logic and will effectively skip it). However, that may not be a big problem, and anything you set in the handler should have no impact - invoking the handler is basically the last thing in a machine.

kolektiv commented 8 years ago

Hmmm. That's interesting, it definitely looks like there's a bug with etags. That certainly needs fixing. v3 is relatively stable (it has various changes and so will result in quite a lot of deprecation warnings, and a few breaking changes to functions, but they're not too surprising). Aside from this current question around lastModified/etags there's nothing major I'm aware of stopping the release, it's purely documentation.

Vidarls commented 8 years ago

Just for clarity: This bug is fixed in v3?

if not: Is there any way to to a manual handler for precondition checking?

Vidarls commented 8 years ago

Also on caching:

Any time a request is received with headers that may cause different representations, the vary header field should be used to ensure any caches does not cache the wrong representation.

This could be media type, language, but also authentication.

Even when cache-control :private is used, multiple users on a shared computer could get a representation from cache that is not intended for them if the logged in user changes. (As cache is keyed by url only by default.

ref: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.44

and: https://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.6

kolektiv commented 8 years ago

If it isn't already fixed in v3, it will be over the next couple of days! A manual handler would be "kind of" possible, but is going to happen at a less than optimal point in the process (you'd have to write it in to the handler, with all of the logic and responses going there, which isn't great).

I'll have a look in to the variation specs (RFCs 7230-7235 are the current ones on this) and make sure we're doing what we can in terms of helping with these scenarios.

Vidarls commented 8 years ago

And it is not super straight forward, more goodness from the RFC:

https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.4

Vidarls commented 8 years ago

I'll try to use the pre-release.

Currently doing a rather big rewrite / refactoring from Proof-of-concept-that-is-currently-in-production to something we can build a production grade solution from, not expecting to put any of this into production until some time next week.

A little note on my project: Biggest challenge I believe will be creating a stack I can easily teach my C# centric team, and making sure the resulting code is fast.

kolektiv commented 8 years ago

Yes, variability isn't completely straight forward at the best of times! I'm going to try and take some time over the next couple of days to make sure that 3.0 is free of known issues and ready to be released at the beginning of next week.

Vidarls commented 8 years ago

So, that was the fastest major version migration I've done ever I think :+1: Clear compiler errors and obsolete warnings

Vidarls commented 8 years ago

But bug is still present in v3:

'Unable to cast object of type myMachine@81-13' to type 'Microsoft.FSharp.Core.FSharpFunc2[Freya.Core.Types+FreyaState,Microsoft.FSharp.Control.FSharpAsync1[System.Tuple`2[Arachne.Http+EntityTag,Freya.Core.Types+FreyaState]]]'.

kolektiv commented 8 years ago

Right, I'll see if I can repro and get a fix done!

Vidarls commented 8 years ago

Quick Q on v3, are there any notes anywhere on subtle breaking changes?

It looks to me like the specs param of handleOk is now removed, may cause some strangeness?

Vidarls commented 8 years ago

Current caching solution, works as intended:

 let handleWithPrivateCache maxAge eTag handler = 
        let cachingHandler handler = 
            freya {
                do! (Freya.Optic.set Response.Headers.cacheControl_ 
                    (Some(CacheControl(
                            [CacheDirective.Private; 
                             CacheDirective.MaxAge(System.TimeSpan.FromSeconds(maxAge))]))))
               do! (Freya.Optic.set Response.Headers.vary_ (Some("Accept Authorization")))
                return! handler
            }
        freyaMachine {
            expires (DateTime.UtcNow.AddSeconds(maxAge))
            //TODO, etag proper when bug fixed
            //etag (ETag(Weak(eTag)))
            handleOk (cachingHandler handler)
        }

usage:

freyaMachine {
    including 
                (handleWithPrivateCache 30.0 (System.Guid.NewGuid().ToString()) 
                 myHandler)}

Composition is fun :-)

Vidarls commented 8 years ago

I have a solution now that mostly works (some OPTIONS issues #186 ) and a bit of ceremony required, but it works. Feel free to close , and maybe open a new issue for a more thorough solution to all things caching if you feel that is wanted.

kolektiv commented 8 years ago

I'll close this one and open a new issue around the possibility of more declarative setting of cache information.