Add support for Touch Interval Rate

[jpfluger]

Looking for some feedback on whether or not this is a good idea. I'd like finer control of Touch(). Why? For a high-volume site, each Touch is a new update to deadline and therefore a new write to the data store. While some handlers will always uses sessions others will not, such as serving static files. Admittedly there are other ways to design this. For example in middleware, I could skip calling Touch for any path serving static files. But a few seconds delay in Touch would serve the same purpose. (And for those static files, I'm still updating global statistics - just not session stats on the static files - which I'm not tracking.)

What if there was a TouchIntervalRate to control how often the deadline gets updated via touch? It could manifest itself a few ways. Arguably the easiest is a new function, session.TouchWithInterval().

// in session.go
// TouchWithInterval func controls the rate at which touches are automatically written via Touch
// there can be quite a few quick touches - which mean writes back to the datastore
// would require a private "isNew" var added to manager.Session
func TouchWithInterval(w http.ResponseWriter, interval time.Duration) error {
	if s.loadErr != nil {
		return s.loadErr
	}

	doTouch := true
	if !s.isNew && interval > 0 {
		timeToTouch := time.Now().Subtract(s.deadline).Add(interval)
		doTouch = timeToTouch.Afer(time.Now())
	}

	if doTouch {
		return Touch(w)
	}

	return nil
}

Thoughts?

[jpfluger]

Another use case is when a website only saves session data for the user and doesn't track http request statistics. That session should be gotten once via Get but not resaved via touch. Maybe in this case (or in addition to the interval), what we need is an IsNew func to tell if the Session has been written to the data store or not.

[jpfluger]

This code I suggested is theory - a number of things wrong with it. I've dug into the actual source to get a working example. Apologies around for my naiveté - part of the learning process with a new repo. I'll post a link here when I have s/thing.

[jpfluger]

I have a working version of TouchWithInterval in a forked repo. From the commit history, you can see there are two underlying issues that needed to be addressed first. Just initiated a pull request for these.

[alexedwards]

Touch is a mechanism for updating the deadline without making any changes to the data. It's primarily used in the Use middleware to update the deadline if - and only if - the user has set an IdleTimeout on the session. You probably know all of that already : )

// There can be quite a few quick Touches that in turn result in writing to the data store.
// This can result in unecessary network traffic. For example, an http request may only
// load a session with user data but it doesn't save any data back to the store. This
// function helps prevent it from happening.

That's exactly the point of Touch. It ensures that the deadline is always updated with each request when IdleTimeout is enabled even if there there is no data changed or saved back to the store. In essence, it restarts the IdleTimeout period whenever some activity is seen from the user, even if the session data itself doesn't change.

My instinct is that for something as critical as session data and timeouts, IdleTimeout needs to be explicit and exacting about keeping a session alive if a user has been seen within a specific window, or timing it out if they haven't. Allowing developers to add an arbitrary interval which means that a deadline may not be updated - even if a user has been active - fundamentally breaks the exact and precise nature of the timeout.

While I understand that there's perfomance drawbacks of this, it's the tradeoff for having an accurate and exact IdleTimeout, which I don't really want to compromise from a security point of view.

[alexedwards]

WRT to the point about calling Touch on static files, I really do think that the correct course of action is also the simplest - just don't use the session middleware on the static file routes.

[jpfluger]

Thank you for the detailed explanation on Touch. That helps settle some of my thoughts.

I now see where my confusion comes from. IdleTimeout is baked into the middleware but I couldn't write the equivalent third party middleware b/c IdleTimeout is private. My middleware had a Touch for every request and nudged me down the path of TouchWithInterval.

I like that you placed the IdleTimeout logic in a different function than Touch(). It keeps Touch() pure. Have you thought about moving the middleware IdleTimeout logic to its own function? Maybe call it CheckIdleTimemout()? Then call that function from the middleware? Along with a corresponding function definition, I think it might be clearer for third party middleware to know it should call that function.

Exposing Opts is enough for anyone to replicate TouchWithInterval inside middleware b/c the saved property would be a value in data. I don't see a use case for it right now but nice to know I could re-implement later.

I think this issue can be closed. I'm going to open up two new ones to discuss other ideas. Thanks for your help on this!