If statement in content_profile_load() not being executed correctly?

marc.groth - September 30, 2009 - 15:04
Project:Content Profile
Version:6.x-1.0-beta4
Component:Base module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs review
Description

OK bare with me, while I try explain my issue.

Currently our site allows certain users to create other users, which works fine. When they are added, depending on the role given to them, certain node types are to be created. These should then be populated accordingly (in code). Oddly I came to realise that although these node types were being created, their details weren't being updated correctly. Upon closer inspection of content_profile_load() I noticed that the if statement (if (!isset($cache[$type][$uid][$lang]))) only gets executed once... Then even if a node ID wasn't returned (or indeed there is no result), it was still being set to blank, which means it IS set, so when the function runs again, it doesn't go into that if statement again. Is this the way it's supposed to work?

The module we have that is utilising this seems to call content_profile_load() a fair bit, but the specific nodes aren't created until the third or fourth call, by which time the if statement doesn't get executed, and FALSE is returned (return !empty($cache[$type][$uid][$lang]) ? node_load($cache[$type][$uid][$lang]) : FALSE;)

Changing the code from:

if (!isset($cache[$type][$uid][$lang])) {
    $cache[$type][$uid][$lang] = FALSE;
    $params = array('type' => $type, 'uid' => $uid);
    if ($node = node_load($lang ? $params + array('language' => $lang) : $params)) {
      $cache[$type][$uid][$lang] = $node->nid;
    }
    return $node;
  }

to:

if (!(int)($cache[$type][$uid][$lang])) {
    $cache[$type][$uid][$lang] = FALSE;
    $params = array('type' => $type, 'uid' => $uid);
    if ($node = node_load($lang ? $params + array('language' => $lang) : $params)) {
      $cache[$type][$uid][$lang] = $node->nid;
    }
    return $node;
  }

(int) instead of isset: seems to make the function work better, in that the if statement gets executed UNTIL a result for $cache[$type][$uid][$lang] is int (i.e. a node ID)

It seems that $cache[$type][$uid][$lang] is set the moment the if statement is executed, REGARDLESS of whether anything is actually returned. Is this the expected behavior? Have I missed something?

You'll have to excuse me, I'm pretty new to all this and it's very likely I'm missing something. Any ideas?

#1

marc.groth - September 30, 2009 - 15:19
Status:active» needs review

I have attached a patch for the change I've made (which makes my code work). Are there any implications as to why this change shouldn't be committed? Or perhaps you can think of a way around it without changing any of the module code? I appreciate any and all ideas/suggestions etc.

Cheers

AttachmentSize
content_profile-6.x-1.0-beta4_content_profile_load_cache.patch 567 bytes

#2

fago - September 30, 2009 - 19:01
Status:needs review» needs work

That's probably a caching issue? Perhaps you'd need to reset the static cache of the function (which is currently not possible)? We should probably add a $reset option to the function.

#3

marc.groth - October 1, 2009 - 10:30

Hi fago,

Thank you for such a prompt response! That makes sense. I've written up something and attached a patch of this... Basically it adds a fourth parameter to the function named $reset (defaulted to FALSE). I then added to the if statement to check for !isset($cache[$type][$uid][$lang]) OR $reset which would be set to TRUE in the function call.

The only thing is now the function call has to physically specify $lang whereas before it could just be ignored. For instance, to set the cache to TRUE it needs to be:

content_profile_load($profile_type, $account->uid, '', TRUE);

whereas before it could be (before my change):

content_profile_load($profile_type, $account->uid);

Can you think of any way that we can have that fourth parameter but not need to specify a third (in the function call I mean). I thought about putting $reset before $lang but then people who have specified $lang will need to change all their function calls accordingly.

Any ideas? :)

Thanks again,

Marc

AttachmentSize
content_profile-6.x-1.0-beta4_content_profile_load_cache_2.patch 854 bytes

#4

marc.groth - October 12, 2009 - 13:30
Status:needs work» needs review

Just realised (bit late haha) that I didn't change the status of this, so doing that now...

 
 

Drupal is a registered trademark of Dries Buytaert.