Flags deactivates page cache in combination with session_api module

I've tested this a small bit. I believe it has something to do with the flag_flag_access function. I had loads of things commented out, and started removing comment wraps. Everything was fine till after I uncommitted this. I'll try to look into a bit more later.

Not the issue. I'm still looking for more.

Found at least one of the problems for certain.

In flag_init, we call flag_set_sid, which checks if session_api module is available, and fires session_api_get_sid()

function flag_set_sid($uid = NULL) {
  ...
  if (!isset($sids[$uid])) {
    if (module_exists('session_api') && session_api_available() && $uid == 0) {
      $sids[$uid] = session_api_get_sid();
    }
    ...
  }
  ...

At the start of that function:

function session_api_get_sid($create = TRUE) {
  ...
  if ($create) {
    // Must initialize sessions for anonymous users.
    session_api_start_session();
  }
  ...

It calls session_api_start_session():

function session_api_start_session() {
  $_SESSION['session_api_session'] = '';
  drupal_session_start();
}

Which drops data into the session.
But, the in the core's session.inc file, a page becomes uncachable the moment anything is added to session:

function drupal_session_initialize() {
  ... 
  if (!empty($_COOKIE[session_name()]) || ($is_https && variable_get('https', FALSE) && !empty($_COOKIE[substr(session_name(), 1)]))) {
    ... 
    // This is where we get buggered
    if (!empty($user->uid) || !empty($_SESSION)) {
      drupal_page_is_cacheable(FALSE);
    }
  ... 

I don't think I know the right solution to the problem, but there's at least part of it.
I'm going to open an issue over on session_api to find out if we really need $_SESSION['session_api'] = TRUE;.

Session_api issue: #1492622: Option to initiate Session API with JS

Right, I think this is more of a Session API problem than a Flag one. The entire reason why we use Session API for anonymous users is to prevent starting anonymous $_SESSION variables.

I'd like to leave this ticket open, for now. I imagine the session_api might make some new feature we have to swap to using instead of the current session_api_start_session(), once it's finished. In the mean time, it documents the known issue of cache death.

The maintainer from session_api checked in for this today. His suggestion was:

Unless I'm missing something, setting $create = FALSE will resolve the issue. If a site needs more control of when sessions are created, that's what the parameter is for. Perhaps flag can make that configurable? I'm also open to how session api itself might resolve this issue, but I don't have any ideas offhand.

I gave that a 20 minute once over. I do retain page caches, and distinct flags per user. Tested with and without js, in two browsers with timestamped template files to confirm.

quicksketch, can you think of anything not directly connected to this issue this could affect? If not, it seems like the solution is as simple as:

if (module_exists('session_api') && session_api_available() && $uid == 0) {
  $sids[$uid] = session_api_get_sid(false);
}

Oh, how simple. Flag indeed is written in such a way that we do not need a session at all. We did a lot of work to make it not dependent upon PHP processing and work with the page cache.

So, the patch is then?

session_api_get_sid(false);

Throw it in dev, and close the ticket?

Okay, after taking a look at this problem, I'm unable to confirm that the Flag module alone causes a problem just by being enabled. As noted above, the call to session_api_get_sid() is what sets a $_SESSION variable ultimately in the Session API module. However the ONLY time Flag module passes in TRUE into that function is inside the flag() function, after an anonymous user has flagged a piece of content.

So the page cache seems to work fine for me, up until an anonymous user flags a piece of content. However at that point, Session API not only sets a "Session API Session" (which has a small auto-increment number), it *also* sets the $_SESSION variable. I don't get what's going on here. The entire reason we use Session API is to *avoid* using $_SESSION. If Session API sets a $_SESSION variable, it disables the page cache. If we weren't concerned about the page cache, Flag module would just use the $_SESSION variable directly and not bother with Session API.

So the current state of things is that the page cache works until an anonymous user flags a piece of content. At that point, Session API puts something in $_SESSION, which disables the page cache. What should happen here instead is that Session API should set a cookie with the Session ID it created so that it can be retrieved.

So, the patch is then?

Doing this will cause Session API to set the SID to "-1" for all anonymous users, which will cause a database error as noted by @Thomas_S.

More research:

The Drupal 7 version of Session API seems to have never worked properly at all. The initial version of Session API included this commit:
http://drupalcode.org/project/session_api.git/commitdiff/81b1a9b2fb99898...

Which includes:

if ($create) {
  // Must initialize sessions for anonymous users.
  session_api_start_session();
}

Which sets the $_SESSION. Oddly that commit credits #785292: Incompatible with Pressflow as the source of the code, which is another issue I participated in that I summarized the problem:

Since no session exists for users in Pressflow or Drupal 7, Session API gets thrown off. We could work around this by starting a session, but then all anonymous users have sessions again and we've lost the benefits of using Pressflow (since users with sessions are not cached by Varnish or Akamai).

It looks like the ultimate solution that was implemented started a session anyway and all page caching was disabled. :(

So basically, this issue is a duplicate of the issue over in Session API: #1492622: Option to initiate Session API with JS

If we split the load of the template of the flag module from the check if is flagged?
The flag module can return the flag template + ajax code to contact separately the server to know the state of the flag (if it is flagged or not).

As in the module jstats that resolve the problem implementing a lightweight version of index.php (that it have the name jstats.php) that collects any statistics information separately.

Best,
hachreak