methods in flag_flag() pass a $uid of 0 to flag_get_user_flags() and cause the cache to miss

Patch includes docs fix and clarification to is_flagged() to state explicitly that the $uid parameter should not be doctored for global flags. I've also added docs to _is_flagged() as that works the opposite way!

Argh, I was sure when I wrote the summary that it made sense and was clear, and now it makes my head hurt :(

Ok so the problem here is that the flag class's method get_flagging_record() tries to be clever with the $uid parameter: if the flag is global, it sets the $uid parameter to zero, whatever you passed in. In a way, that makes sense, because in the database, a global flag's flaggings all have uid set to 0.

However, this 0 is passed to flag_get_user_flags().

Now flag_get_user_flags() has an internal static cache, which is loaded up once per user/entity combination. In other words, the first time you visit this for a given user and entity, data for this user and entity for ALL flags is loaded into the static cache:

    if (!isset($flagged_content[$uid][$sid][$entity_type][$entity_id])) {

So what can happen is this:

- Current user is logged-in, so uid is non-0.
- First flag on the node is global.
-- Flag link rendering needs to know what to render, so it calls is_flagged(), which calls get_flagging_record()
-- get_flagging_record() hacks the $uid to 0 because the flag is global
-- flag_get_user_flags() gets the $uid of 0, and so populates the internal cache at the 0.
- Second flag on the node is non-global
-- Flag link rendering needs to know what to render, so it calls is_flagged(), which calls get_flagging_record()
-- get_flagging_record() does NOT hack the $uid to 0 because the flag is non-global
-- flag_get_user_flags() gets the real $uid. It has no cache data for that, so has to query again: so we've had a cache miss, and we've had to query {flagging} twice instead of once.

Spotted something else:

      $result = db_select('flagging', 'fc')
        ->fields('fc')
        ->condition('entity_type', $entity_type)
        ->condition('entity_id', $entity_id)
        ->condition(db_or()
          ->condition('uid', $uid)
          ->condition('uid', 0)
        )
        ->condition('sid', $sid)
        ->execute();

Suppose we have a global and a non-global flag. The query above has a condition on the $sid, and for a global flag the sid column is always 0. This is fine if we're not using Session API, as without that module a user's $sid is always 0. But if we do have that, then the query won't work.

I'm not sure if it means that the static cache simply can't always cover everything, or that we have another bug here.

EDIT: we do, and I filed it already here: #2487003: possible bug with retrieving data for anon-flagged global flags in flag_get_user_flags().

Hmm so it looks like the fixing the bug we're fixing here will expose this bug in the query, and we might actually get flags that don't render correctly.

So I'm going to remove the blocker tag from this, as d.org people have requested a new release for Flag and this one requires more thought.