On Tue, May 31, 2011 at 13:21, Rodrigo Rosenfeld Rosas <[email protected]>wrote:
> Em 31-05-2011 08:07, Christian Johansen escreveu: > > > > On Tue, May 31, 2011 at 12:48, Rodrigo Rosenfeld Rosas <[email protected] > > wrote: > >> Em 31-05-2011 07:42, Christian Johansen escreveu: >> >> On Tue, May 31, 2011 at 12:22, Rodrigo Rosenfeld Rosas < >> [email protected]> wrote: >> >>> Em 31-05-2011 06 <31-05-2011%2006>:31, Christian Johansen escreveu: >>> >>> On Fri, May 27, 2011 at 10:40, Christian Johansen < >>> [email protected]> wrote: >>> >>>> >>>>> >>>>> But since you're asking so critically I have to admit I'm not entirely >>>>> sure if the session check is entirely required. However, it did seem to be >>>>> the thing that caused all the tests to fail? Maybe Marius has better >>>>> memory >>>>> than me here? >>>>> >>>>> The only case we could think of that uses the session without the >>>> user being logged in is in the case of flash messages. However, flash >>>> messages are only used in actions that are POST-ed to, in which case the >>>> user should be on https already. So I think we can remove that part. >>>> >>>> I will fix this method on master. >>>> >>> >>> Finally remember the actual reason why we have the session expiry >>> check. The reason is caching. Rails is very eager when it comes to session >>> cookies. Basically as soon as you touch the session object, Rails _will_ >>> send a cookie. The cookies prevent us from utilizing our cache frontends >>> properly. So in this case, merely checking that the user is logged in will >>> cause a session cookie to be sent. I ended up with this: >>> >>> def using_session? >>> !request.session_options[:expire_after].nil? >>> end >>> >>> def ssl_allowed? >>> request.ssl? >>> end >>> >>> def ssl_required? >>> GitoriousConfig["use_ssl"] && using_session? && logged_in? >>> end >>> >>> Which at least reads clearer to me. What do you think? >>> >>> >>> Much clear undoubtedly! But still, if caching is a concern here, this >>> should be stated at least as a comment there. >>> >> >> Good point: >> >> # "Safely" check whether or not we're using the session. Unfortunately >> # simply touching the session object will prompt Rails to issue a >> session >> # cookie in the response, which in some cases breaks caching. >> # >> # Use this method as a guard in actions where cacheability is >> important, >> # and you most probably don't need to access the session. >> def using_session? >> !request.session_options[:expire_after].nil? >> end >> >> As far as I know, this issue is much easier to handle under Rails 3, >> which isn't as eager on sending session cookies. >> >> >> The explanation is great! >> >> And have you checked whether this is really avoiding Gitorious from >> sending a cookie? It is always sending the cookie in my tests. >> > > That's because most actions still touch the session. We have done some > work with a few often used actions to be able to cache them, but this is > just a start. Here's an example: `curl -i > http://gitorious.org/gitorious/mainline/config` > > > Right, but shouldn't the root mapping be cacheable too? > As long as you're not logged in, sure. But as I said, we've just recently started with this, and focused on the most frequently hit actions first. The config action is used for all git operations, thus was one of the really important ones to cache. Another is the commit view, where we now pull in the diff and comments with JavaScript, allowing them to be cached heavily (this saved us quite a bit). Christian -- To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected]
