I can reproduce this on Drupal 7.12, Flags 7.x-2.0-beta6 and Session API 7.x-1.0-beta2 (or 7.x-1.x-dev):
- install flags module -> drupal page cache is ok
- install session_api module -> drupal page cache was deactivated , even if anonymous flagging is disabled. And even if all flags are deleted.
(delete cookies in browser -> the next page was cached for this moment, but click on an internal link of the site and the cache is disabled again)
- delete flags module -> drupal page cache is now ok. The combination of flags and session_api module deactivates the page cache, even if anonymous flagging is deactivated?
Comments
Comment #1
Thomas_S CreditAttribution: Thomas_S commentedComment #2
lance.gliser CreditAttribution: lance.gliser commentedI'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.
Comment #3
lance.gliser CreditAttribution: lance.gliser commentedFound 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()
At the start of that function:
It calls session_api_start_session():
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:
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;.
Comment #4
lance.gliser CreditAttribution: lance.gliser commentedSession_api issue: #1492622: Option to initiate Session API with JS
Comment #5
quicksketchRight, 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.
Comment #6
lance.gliser CreditAttribution: lance.gliser commentedI'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.
Comment #7
lance.gliser CreditAttribution: lance.gliser commentedThe maintainer from session_api checked in for this today. His suggestion was:
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:
Comment #8
quicksketchOh, 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.
Comment #9
lance.gliser CreditAttribution: lance.gliser commentedSo, the patch is then?
Throw it in dev, and close the ticket?
Comment #10
Thomas_S CreditAttribution: Thomas_S commentedPatch session_api_get_sid(false) in flag.module lets the cache active.
But if I flag a node with this patch, it comes a http 500 "service unavailable" Error.
Ajax script cant write to DB because the sid = -1
PDOException: SQLSTATE[22003]: Numeric value out of range: 1264 Out of range value for column 'sid' at row 1: .....
becausxe the sid = -1
[:db_insert_placeholder_4] => -1
Comment #11
Thomas_S CreditAttribution: Thomas_S commentedplease can somebody help with a patch to fix the anonymous flagging problems ?
Comment #12
quicksketchOkay, 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.
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.
Comment #13
quicksketchMore 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:
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:
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
Comment #14
hachreak CreditAttribution: hachreak commentedIf 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