[Zope-dev] Zope 2.4.0 CopySupport changes seem wrong

[Martijn Faassen]

Hi there,

I've just tracked down a bug that suddenly appeared with Formulator used
on Zope 2.4.0. The problem is with renaming fields (which are objects
in an ObjectManager).

Formulator builds on ObjectManager and overrides manage_afterAdd and
manage_beforeDelete to track its own list of object ids, which are
kept in a certain order. Formulator offers special management
facilities for this.

The Field's manage_afterAdd and manage_beforeDelete pass on the id of the
changed field to the order management functions of the form. It turns
out however that manage_renameObject in OFS/CopySupport.py was changed,
from this:

        self._delObject(id)
        ob = aq_base(ob)
        ob._setId(new_id)
        
        # Note - because a rename always keeps the same context, we
        # can just leave the ownership info unchanged.
        self._setObject(new_id, ob, set_owner=0) 

to this:


        b = aq_base(ob)
        ob._setId(new_id)
        
        # Note - because a rename always keeps the same context, we
        # can just leave the ownership info unchanged.
        self._setObject(new_id, ob, set_owner=0)
        self._delObject(id)

Before self._delObject is moved to below ob._setId(), manage_beforeDelete
receives an object as its 'item' parameter that has the changed id already.
This seems wrong to me. What was the reason manage_renameObject() was changed
this way?

Checking CVS:
 
[cvs.zope.org has an unfindable stylesheet today, by the way, at least
with Netscape 4.7 on Linux; I got to the viewcvs interface only
by turning off stylesheets in Netscape.]

Hm, apparently it was an attempt to fix this collector item:

http://classic.zope.org:8080/Collector/2257/view 

How would I implement code that removes the original id of a deleted
object from say, a list of ids, in the new situation? There's no way
to know now what the old id was. :) 

The bug report appears a big strange; it mentions an 'manage_afterDelete'
which apparently doesn't exist (does the ZClass introduce this functionality?)
I don't really understand how the previous functionality was wrong
in the first place from this or the CVS log entry.

If you're going to manage external resources with manage_beforeDelete, 
manage_afterAdd ought better introduce them again afterwards if you do
a rename. Reordering this to make manage_afterAdd occur *before* 
manage_beforeDelete seems wrong to me.

Regards,

Martijn