By nightowl77 on

Hi Guys

I'm following along in the Pro Drupal Development book and noticed something strange. I'm currently on page 127 and implemented the "loginhistory" module they demo in the book.

I added "drupal_set_message('User load hook in loginhistory called');" and noticed that hook_user $op = load is called 7 times. This means 7 unnecessary queries to the database.

This is the loginhistory.module code 

function loginhistory_user($op, &$edit, &$account, $category = NULL)
{
	//drupal_set_message('Operation is '.$op . ' category '. $category );
	switch($op)
	{
		case 'login':
			// Record tempstame in database
			db_query("INSERT INTO {login_history} (uid, login) VALUES (%d, %d)",
				$account->uid, $account->login
			);
			
			break;
			
		// $user object has been created and is given to us as $account parameters
		case 'load':
			// Add the number of times the user has logged in
			$account->loginhistory_count = 
				db_result(
					db_query(
						"SELECT COUNT(login) AS count FROM {login_history} WHERE uid = %d",
						$account->uid
					)
				);
				drupal_set_message('User load hook in loginhistory called');
				
			break;
			
			
		// 'My account' page is being created
		case 'view':
			// Add a field displaying number of logins
			$account->content['summary']['login_history'] = array(
			  '#type' => 'user_profile_item',
				'#title' => t('Number of Logins'),
				'#value' => $account->loginhistory_count,
			  '#attributes' => array('class' => 'login-history'),
				'#weight' => 10
			);
			
			break;
	}
}

When I go to the profile page, I get this in the drupal message feedback block:

User load hook in loginhistory called
User load hook in loginhistory called
User load hook in loginhistory called
User load hook in loginhistory called
User load hook in loginhistory called
User load hook in loginhistory called
User load hook in loginhistory called

Am I missing something here? Is there a reason why the drupal core would call the hook_user('load'...) so many times?

I checked this on other pages (non user profiles) and it only fires the hook_user('load'...) once.

I have the profile module installed - could this module cause this?

Thank you for your help guys (hope this isn't because of a mistake I made)

Richard

⋅ Categories: Drupal 6.x

Comments

savioret’s picture

savioret commented

I have the same problem.
Are you using ctools ?

I've been debugging and this is the result


menu_get_item [menu.inc] (ln 343)
   menu_translate (ln 316)
     _menu_load_objects (ln 566)
        *callback* user_uid_optional_load
	  <b>user_load</b> 
	    user_module_invoke('load')
              myhook_user
  user_view
    user_build_content
      user_module_invoke('view')
        myhook_user
print theme('page', $return); [index.php] (ln 36)
  theme('blocks', 'left')
    theme('block', 'left')
      module_invoke('user_block', 'view')
        menu_tree('navigation')
          menu_tree_check_access  // Check access for the current user to each item in the tree.
             _menu_link_translate
              _menu_load_objects
                *callback* user_uid_optional_load
                  <b>user_load</b>
                     user_module_invoke('load')
                        myhook_user
  template_preprocess_page
    theme('help')
      ctools_menu_help
        ctools_menu_get_active_help
           ctools_menu_tab_root_path
             ctools_menu_local_tasks
              _menu_translate
                _menu_load_objects
                   <b>user_load</b>
                      user_module_invoke('load')
                         myhook_user
              _menu_translate
                _menu_load_objects
                   <b>user_load</b>
                      user_module_invoke('load')
                         myhook_user
              _menu_translate
                _menu_load_objects
                   <b>user_load</b>
                      user_module_invoke('load')
                         myhook_user
              [...] about 20 more times

Seems that ctools is calling menu_translate multiple times, each one calling user_load ...
I don't know if there's any other way to do that, maybe I this should be reported to the ctools module.

I have opened an Issue in the Ctools project http://drupal.org/node/557076

asiby’s picture

asiby commented

And I am not using ctools. Just my own custom module trying to implement hook_user(..load..).

Live long ... and prosper!

asiby’s picture

asiby commented

I am having the same thing without ctools. Just my own custom module trying to implement hook_user(..load..).

Live long ... and prosper!

asiby’s picture

asiby commented

I am having the same thing without ctools. Just my own custom module trying to implement hook_user(..load..).

Live long ... and prosper!

extexan’s picture

extexan commented

Ditto for me!

Can someone explain why this is happening? Please???

It's never too late to have a happy childhood. ;-)

greenbeans’s picture

greenbeans commented

Subscribing. It doesn't seem to break anything, but isn't exactly kind to database load.

alexkb’s picture

alexkb commented

I'm seeing the same problem too. Digging around a little further, according to this post, its not going to be fixed in Drupal 6:
#685806: hook_user op 'load' called multiple times on user account page; queries run needlessly

The patch for Drupal 7 is here:
#347250: Multiple load users

jaypan’s picture

jaypan
Location Victoria, BC
commented

The original poster's code could be optimized using the following:

function loginhistory_user($op, &$edit, &$account, $category = NULL)
{
    //drupal_set_message('Operation is '.$op . ' category '. $category );
    switch($op)
    {
        // only showing case load 
        case 'load':
            // Add the number of times the user has logged in
            $account->loginhistory_count = get_login_history_count($account->uid);
            break;
    }
}

function get_login_history_count($uid)
{
  static $login_history_count;
  if(!$login_history_count)
  {
    $login_history_count = db_result(
       db_query(
          "SELECT COUNT(login) AS count FROM {login_history} WHERE uid = %d",
          $uid
       )
    );
    drupal_set_message('User load hook in loginhistory called');
  }
  return $login_history_count;
}

This would only optimize this module - so any other modules may be repeating their db calls multiple times, but it would prevent at least this one query from being executed multiple times.

Contact me to contract me for D7 -> D10/11 migrations.