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.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 605594.patch | 1.05 KB | ezra-g |
Comments
Comment #1
markus_petrux commentedActually, 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:
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:
Comment #2
markus_petrux commentedAdjusting issue title.
Comment #3
OliverColeman commentedWow, 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!
Comment #4
karens commentedI 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.
Comment #5
karens commentedI did not mean 'nodes are not created', I meant 'dates are not created'. The nodes are created without dates.
Comment #6
iva2k commentedsubscribing
Comment #7
bibo commentedWorks 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().
I'll check if that works.
Comment #8
markus_petrux commented@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.
Comment #9
bibo commentedThanks for clearing that up. This fix is really needed.
Comment #10
ezra-g commentedI 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.
Comment #11
dwwIt'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 ;)
Comment #12
ezra-g commentedThis is committed. Thanks!
http://drupal.org/cvs?commit=435112
Comment #14
ezra-g commentedComment #15
greggles6.x-2.x is no longer supported. closed (fixed) for 6.x-1.x.
Comment #16
dww