Page cache should work for anonymous users with non-empty $_SESSION

This includes some minor cleanup of comments.

Not quite convinced it can be this simple...!

#322104: Allow hook_boot() to disable page cache would also be a useful further extension since then, for example, something like a shopping cart could be displayed to anonymous users who have something in it, but other anonymous users could continue to see cached pages.

Thanks moshe, should I try to track 'em down in IRC????

Slightly modified patch.

I think this could be useful for things like http://drupal.org/project/affiliate, which stores referral information in the session. Also for a requirement we have for anonymous sessions #591348: How to enable anonymous sessions.

Actually #322104: Allow hook_boot() to disable page cache would also be *required* if the current behavior introduced in 7.x (where users with data stored in $_SESSION never get a cached response) is ever needed. A contrib module could then easily examine $_SESSION in hook_boot() and decide whether to force a non-cached response.

Umm... unless I've misunderstood the suggestion, this cache invalidation is being done during bootstrap *before* trying to get the page from the cache with http://api.drupal.org/api/function/drupal_page_get_cache/7 ...

Also this is cache invalidation of the "don't serve this request from the cache" variety, rather than the "don't save this page into the cache" variety, drupal_page_is_cacheable() seems to be already used for both.

>prevent a page from being served from the cache as soon as another version of the same page has been cached
#322104: Allow hook_boot() to disable page cache, which would allow the cart module to revert Drupal to the behavior currently in core by doing this in hook_boot():

if (isset($_COOKIE[session_name()])) {
  drupal_page_is_cacheable(FALSE)
}

Or it could boot up to DRUPAL_BOOTSTRAP_SESSION phase and actually examine $_SESSION to determine if the cart is empty or not.

Let's combine the patch at #322104-18: Allow hook_boot() to disable page cache into this one and then repurpose the other issue to look at adding additional parameters to the page cache key as you suggested.

New patch that also allows modules to use hook_boot() prevent the current page request from being served from the cache. Seems to do what it says on the tin with respect to hook_boot().

The only sub-optimal thing I can find is that if you have an anonymous cookie which then gets removed and subsequently you get a cache hit for a page you viewed while you had the cookie you get a 200 cached response rather than a 304 cached response, because of Vary: Cookie, but that was not introduced by this patch.

Dear me, standards slipping ;)

Thanks for dropping by, c9..., and checking this out.

Sorry, you're right, not making myself clear.

What I meant to say was that I tested calling drupal_page_is_cacheable(FALSE) from within a module's hook_boot() and confirmed that this disabled the page cache for the current request (in answer to Damien's #7). I tried a couple of variants - unconditionally calling drupal_page_is_cacheable(FALSE), and conditionally calling it depending on the local path and also depending on whether isset($_COOKIE[session_name()]), thereby replicating the current behavior of core. These variants were probably unnecessary but confirmed in my mind that everything was behaving as it should (and actually highlighted another minor problem: #592482: Regression: data stored in $_SESSION in hook_boot() or hook_exit() during a cached page response is lost).

Yes I think I can see where you are coming from in that if something is stored in $_SESSION then presumably you'd want to do something with it (e.g. modify the page output) and that can't be done if you are serving cached pages.

Our specific use case is not so much concerned with the content of $_SESSION but using the session to track site visitors. We use this for optimising AdWords campaigns since it's possible to associate individual sessions (and hence numbers of pageviews, registrations) with keywords etc. This supplements the analysis available from within AdWords and Google Analytics.

http://drupal.org/project/affiliate appears to do something similar - it stores referral information in the session, but this remains latent until e.g. the visitor actually signs up.

Different approach: default Drupal 7.x behavior is unchanged, but you can override the logic that determines whether the current page may be served from the cache or saved to it, by defining custom functions which will be invoked if they exist (à la custom_url_rewrite...).

Not sure what the merits of this are .. except that it should make chx's developer experience less terrible.. ;) I should say the naming of the "custom..." functions fills me with some revulsion.

Probably the !empty($user->uid) in drupal_session_initialize() should be put inside the scope of the custom behavior, to be more symmetric with custom_check_if_page_may_be_served_from_cache(). The latter has to include the necessary logic that prevents cached pages being served to logged-in users (the default isset($_COOKIE[session_name()] covers this case plus any non-empty session in one fell swoop). Of course this means that if the custom cache checks are done wrongly you could end up serving pages from or saving pages to the cache for logged-in users, but there seems no way of absolutely preventing developers from getting this wrong.

With this patch I can get the specific behavior I want by adding the following to settings.php (a custom cache.inc would also work):

/**
 * Enable page cache for anonymous users with non-empty $_SESSION.
 */
function custom_check_if_request_may_be_served_from_cache() {
  // We don't know yet if the user is anonymous or not.
  if (isset($_COOKIE[session_name()])) {
    // Need to check the session ... initializing the session in effect checks
    // whether the page may be served from cache :-) ... first of all checking for
    // $user->uid and possibly invoking custom_check_if_page_may_be_saved_to_cache(),
    // and calling drupal_page_is_cacheable(FALSE) if appropriate. This
    // blocks the serving of a cached page in drupal_page_get_cache().
    drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION, FALSE);
  }
  // Always return TRUE: if the request should not be served from the cache
  // then drupal_page_is_cacheable(FALSE) will have been called from
  // session_initialize().
  return TRUE;
}

function custom_check_if_page_may_be_saved_to_cache() {
  // If the user is logged in then the saving of the page to the cache will
  // already have been prevented. For anonymous users we are happy to save the
  // page to the cache if $_SESSION is non-empty, unless there are status
  // messages to display. 
  return count(drupal_get_messages(NULL, FALSE)) == 0;
}

Ahhhhh trying again!

Adding tag, though it seems not many other people are affected/concerned by this..!

chx doesn't like #11, and I don't really like #16/#17. Another approach would be to have a simple variable that could be set in settings.php (e.g. 'page_cache_with_anonymous_sessions') that is FALSE by default (giving the current behavior) and if set to TRUE gives the current 6.x behavior.

Whether I get round to rolling a patch and whether there is time for it to be reviewed before 1 Dec remains to be seen :P

And this is even later!

Implements #19. When variable 'page_cache_with_anonymous_sessions' is not set there is no change from current behavior and anon visitors never get a cached response if something is stored in their $_SESSION.

When variable 'page_cache_with_anonymous_sessions' is set to TRUE in settings.php then anon visitors can receive a cached response even with a non-empty session, but not if a status message is displayed. Logged in users never get a cached response.

[udpate: just to note that Firebug 1.4.5 running on Firefox 3.5.5 displays the X-Drupal-Cache header even for a 304 response. Obviously with a 304 response you must have hit the cache, and indeed Drupal tries to send the X-Drupal-Cache: HIT header again, but Firebug prefers to show the cached copy of the header which may say MISS. Don't let this confuse you ...! Live HTTP headers on the other hand doesn't show this header when a 304 is received.]

@26: #21 might do what you want, by letting you define

$conf['page_cache_with_anonymous_sessions'] = TRUE;

in settings.php, which lets Drupal use the page cache for (non-empty) anon sessions, as in 6.x. However although this variable could be set conditionally, it might be hard to do so intelligently this early in the bootstrap; you'd probably have to do some tests in cache.inc where you could raise the boostrap level without triggering the page cache. BTW, I'm sure I'm teaching granny to suck eggs ;)

@23:
>If you want/need to do tracking, use another cookie
Well yes, I was rather coming to the same conclusion. Much of my reluctance being 1) statistics.module +Views pretty well does what's needed in 6.x (I found this surprisingly useful and identified various usability problems on one site this way); and 2) I have little understanding (yet) of cookie handling! Maybe it's not so hard.

>Agreed that "blindly" cacheing these pages is a bad idea.
Well at least we all agree there!!