Remove buddylist_get_buddies from hook_user load $op

I see the issue here, and this clearly has to get solved for this module to scale.

However, tearing it out will likely break a lot of sites that depend on it for simple functionality, so this change would likely need to be acoompanieid by a branch and some documentation.

I'd like to be able to make this change. Perhaps as part of a new branch to begin working towards a better/stronger/faster drupal 6 version?

Hi Gang,

Sorry I've dropped of the buddylist planet for quite awhile - I've seen this issue before and I removed that code from my own local copy. What about making this a variable_get() driven behavior that way it can be disabled yet preserve the API?

I'm mostly commening because I see mention of doing a Drupal 6 version - has something changed with buddylist 2 - I was sort of under the impression this version of the module was going to fade away. If plans are being made to do a new version of this code for 6 I'd be interested and I'd suggest a complete rewrite. This code base is a mess and could be way better and simpler.

Cheers!

Following up better late than never, I actually don't see this. There's no user_load in buddylist_get_buddies(), which would be the cause of such recursion:

function buddylist_get_buddies($uid = NULL, $key = 'uid') {
  static $buddies;

  if (!$uid) {
    global $user;
    $uid = $user->uid;
  }
  if (!isset($buddies[$key][$uid])) {
    $buddies[$key][$uid] = array();
    $sql = 'SELECT b.buddy, u.name, u.mail, u.uid FROM {buddylist} b
            INNER JOIN {users} u ON b.buddy = u.uid
            WHERE b.uid = %d';
    $result = db_query($sql, $uid);
    while ($row = db_fetch_object($result)) {
      $buddies[$key][$uid][$row->buddy]['uid'] = $row->uid;
      $buddies[$key][$uid][$row->buddy]['name'] = $row->name;
      $buddies[$key][$uid][$row->buddy]['mail'] = $row->mail;
      if (variable_get('buddylist_buddygroups', FALSE)) {
        $buddies[$key][$uid][$row->buddy]['groups'] = buddylist_get_buddy_groups($uid, $row->buddy);
      }
      $buddies[$key][$uid][$row->buddy]['online'] = 0;
      $selectlist .= $row->buddy.",";
    }
    // Add the online flag
    if (db_num_rows($result)) {
      $sql = 'SELECT uid FROM {sessions} WHERE uid IN (%s) AND timestamp > %d';
      $result = db_query($sql, substr($selectlist,0,-1), time()-1800);
      while ($row = db_fetch_object($result)) {
        $buddies[$key][$uid][$row->uid]['online'] = 1;
      }
    }
  }

  return $buddies[$key][$uid];
}

I'll keep an eye on this as we work towards 6.0 though. Please update me if I'm wrong here.

This is definitely an issue. On our site, there was no way to keep it live without doing this:

function buddylist_user($type, &$edit, &$thisuser, $category = NULL) {

....
else if ($type == 'load') {
/** @logomod, LOGOT-349: Kill the module invoke all hook_user load cycle for the buddylist module so a user's buddies aren't loaded every time the user is loaded.
$thisuser->buddies = buddylist_get_buddies($thisuser->uid);
*/
}
...

The things is that for every buddy of user A, each buddy triggers the hook _user with $type 'load', which means that for each and every buddy of user A, all the buddies of user A's buddies also get loaded. There's a non-linear collapse of scalability here.

Huge +1 from me.