[drupal-devel] [bug] Make node_save() recursion-aware

[Dries]

Issue status update for 
http://drupal.org/node/27007
Post a follow up: 
http://drupal.org/project/comments/add/27007

 Project:      Drupal
 Version:      cvs
 Component:    node system
 Category:     bug reports
 Priority:     normal
 Assigned to:  jvandyk
 Reported by:  jvandyk
 Updated by:   Dries
 Status:       patch (ready to be committed)

Won't commit.  If you are calling node_save() recursively, something is
really wrong.  For one, you can no longer guarantee that the node is in
a 'safe state'. Like, what happens if you saved half a node and you
recursively try saving that node again?  This needs a better solution.




Dries



Previous comments:
------------------------------------------------------------------------

Tue, 19 Jul 2005 02:14:16 +0000 : jvandyk

Attachment: http://drupal.org/files/issues/node_serialized2x.patch (664 bytes)

node_save() saves a node, then calls insert or update hooks. These hooks
may run functions that, in turn, call node_save().


This all works fine except that if a revision is being created, the
revision gets serialized twice -- once on the original node_save() call
and once on the callback's node_save() call.


The attached two-line patch simply inserts a boolean variable called
rev_serialized to keep track of whether the serialization has already
been done.


I'd like to see cvs and 4.6 patched.


This patch is necessary so I can release actions and workflow for 4.6.


Background: http://drupal.org/node/24326




------------------------------------------------------------------------

Tue, 19 Jul 2005 18:36:23 +0000 : Dries

I don't like this patch.  It's a hairy fix.  There must be a better way
to avoid recursion in first place.




------------------------------------------------------------------------

Thu, 18 Aug 2005 01:26:21 +0000 : moshe weitzman

gosh, this a two line change with zero performanhce impact. seems like a
useful and minimally invasive fix for our current dilemma. we'll remove
it once a better fix comes around. is a shame to hold back
workflow/actions for such a minor issue. any others want to comment or
suggest a different solution?/




------------------------------------------------------------------------

Thu, 18 Aug 2005 04:57:11 +0000 : chx

If there would be a little comment why this is needed, then I'd +1 it
heartily.




------------------------------------------------------------------------

Thu, 18 Aug 2005 11:59:14 +0000 : killes at www.drop.org

Please commit. In case revisions shouldn't make it (for performance
reasons ;) we need it fixed for 4.7.




------------------------------------------------------------------------

Thu, 18 Aug 2005 14:22:15 +0000 : moshe weitzman

The first user of this will be workflow+actions modules. They listen on
nodeapi's insert/update operations. When a node is seen there,
arbitrary actions may fire according to the admin's wishes. So for
example, a node may be promoted or may become sticky. To do that
properly, the actions should call node_save() again with
$node->promote=1. Thus, we are recursively calling node_save() which
leads to a double serialize problem. This 2 line fixes it.


There may be more nodes which benefit from this.




------------------------------------------------------------------------

Thu, 18 Aug 2005 14:23:24 +0000 : moshe weitzman

I meant to say - "there may be more *modules* which benefit from this."