Here's an issue that has been bugging for a while.

I'm using filefiels_paths for an imagefield where the filepath uses a custom token. This means that filefield_paths will move the file to the final location and update the record in the {file} table to reflect the new filepath.

On the other hand, during node_load(), CCK caches the data of the fields, and it happens that filefield loads the record of the {file} table here, which is then cached by CCK.

When a node is updated, CCK clears the cache entry, but when a node is created, if something in your system causes a node load after the node has been saved by node module, but before filefield_paths updates the record in the {file} table, it will happen that this node_load() will make CCK cache an incorrect version of the file data, because it has not been updated yet by fieldfield_paths.

The solution to this is pretty simple. Just make sure the CCK cached version of the node is cleared if filefield_paths_file_move() succeeds.

I'm not 100% sure this is the best way to fix, but a quick one is:

       if (filefield_paths_file_move($file)) {
+        cache_clear_all('content:'. $node->nid .':'. $node->vid, content_cache_tablename());

I don't know what causes a node_load() between the node has been created and this step in filefield_paths, but it seems to me it was signup module in my case, in signup_save_node(). I guess it could also happen from other modules, so this is why I'm reporting this here. If the CCK cache is cleared here, then no matter if other modules do a node_load() for whatever reason.

CommentFileSizeAuthor
#10 605594.patch1.05 KBezra-g

Comments

markus_petrux’s picture

Project: File (Field) Paths » Signup
Version: 6.x-1.3 » 6.x-1.0-rc6
Status: Active » Needs review

Actually, the fix above does not work because signup executes after filefield_paths module, so the node_load() performed by signup module causes the node data to be cached by CCK after the fix above happens.

I'm moving this issue to the signup queue, as I have finally found a method to fix this in that module. I'm using 6.x-1.0-rc6 of signup module, and the following change to signup/includes/node_form.inc, function signup_save_node() fixed the issue here:

   $node = node_load($node->nid);
+  if (module_exists('content')) {
+    cache_clear_all('content:'. $node->nid .':'. $node->vid, content_cache_tablename());
+  }
   if (_signup_node_completed($node) && !empty($node->signup_status)) {

In the meantime, I'm using a custom module that I use for hacks, that has a weight higher than other modules, and here I'm implementing the following hook_nodeapi(), that also works:

/**
 * Implementation of hook_nodeapi().
 */
function MODULE_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) {
  switch ($op) {
    case 'insert':
    case 'update':
      // Ensure content cache is cleared to account for changes in {file}
      // records when these have been updated by filefield_paths module
      // using tokens, broken by signup module doing a node_load() during
      // a node_save() operation.
      // See http ://drupal.org/node/605594
      cache_clear_all('content:'. $node->nid .':'. $node->vid, content_cache_tablename());
      break;
  }
}
markus_petrux’s picture

Title: Clear content cache after updating {file} data » node_load() during signup's node_save() breaks node data cached by CCK

Adjusting issue title.

OliverColeman’s picture

Wow, this is nasty... I have several different content types with various types of CCK fields, and whenever trying to create a new node for any of those types (regardless of whether they were signup enabled) the CCK fields wouldn't save (but would when editing them).

The first patch in #1 seems to have fixed the problem, thanks!

karens’s picture

Priority: Normal » Critical

I can confirm that the above fix works and that nodes are not created without it. In my case, I am trying to create new dates from the calendar 'Add+' link. It appears to work, but the date is not saved without the above fix. With the above fix everything works fine.

karens’s picture

I did not mean 'nodes are not created', I meant 'dates are not created'. The nodes are created without dates.

iva2k’s picture

subscribing

bibo’s picture

Status: Needs review » Reviewed & tested by the community

Works for me too! Commit this please.

EDIT: oops, actually I ment to write this to the other issue related to this: http://drupal.org/node/550036

Btw, afaik clearing all caches is overdoing it. Just clearing the node cache should be enough, and that can be done with the 3rd parameter of node_load().

$node = node_load($node->nid, null, true);

I'll check if that works.

markus_petrux’s picture

@bibo: Nope, the static cache in node_load() does nothing here because the problem is CCK caches the node in its own cache table {cache_content}. We need to clear the entry created by CCK in that table for the node being processed here.

bibo’s picture

Thanks for clearing that up. This fix is really needed.

ezra-g’s picture

Version: 6.x-1.0-rc6 » 6.x-1.x-dev
StatusFileSize
new1.05 KB

I wasn't able to reproduce this, but given the severity of the problem and that it apparently contributes to issues like http://drupal.org/node/550036#comment-2190634, and with a +1 from KarenS, I'd like to commit. I've rolled this as a patch that also includes a comment.

Would love to get dww's +1 on this, but will commit soon either way.

dww’s picture

It's a bummer you can't reproduce this. Probably makes it harder to write a simpletest for it. ;) But if KarenS is happy, I'm happy. Correctness is more important than performance.

+1 ;)

ezra-g’s picture

Status: Reviewed & tested by the community » Fixed

This is committed. Thanks!

http://drupal.org/cvs?commit=435112

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

ezra-g’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev
Status: Closed (fixed) » Patch (to be ported)
greggles’s picture

Status: Patch (to be ported) » Closed (fixed)

6.x-2.x is no longer supported. closed (fixed) for 6.x-1.x.

dww’s picture

Version: 6.x-2.x-dev » 6.x-1.x-dev