[Zope-dev] Re: CopySupport, hooks, events

[Florent Guillaume]

Geoff Davis wrote:
> I've been talking to people on #zope about this.  They're scared to
> touch the methods for backward compatibility reasons.
> 
> Maybe the way to go is to create some new methods that are called
> recursively on objects when they are copied or moved.  How about:
> 
> def _beforeMove(self, 
> 		destination_container, 
> 		destination_path,
> 		recurse=True):
>    # destination_container may be None (since for subobjects, the
>    #    destination will not have been created yet)
>    # destination_path is a tuple containing the physical path of
>    #    the new container (which may not yet exist)
>    # call subobjects recursively if recurse flag is True
>    #
>    # default implementation does nothing and returns self
> 
> def _afterMove(self, source_container, recurse=True)
>    # source_container may be None (since for subobjects, the
>    #    source may have been moved elsewhere)
>    # source_path is a tuple containing the physical path of 
>    #    the old container (which may no longer exist) 
>    # call subobjects recursively if recurse flag is True 
>    #
>    # default implementation does nothing and returns self 
> 
> def _beforeCopy(self, destination_container, recurse=True):
>    # destination_container may be None (since for subobjects, the
>    #    destination will not have been created yet)
>    # destination_path is a tuple containing the physical path of
>    #    the new container (which may not yet exist)
>    # call subobjects recursively if recurse flag is True
>    #
>    # default implementation does nothing and returns self
>  
> def _afterCopy(self, source_container, original_object, recurse=True):
>    # source_path is a tuple containing the physical path of 
>    #    the new container (which should still exist) 
>    # call subobjects recursively if recurse flag is True 
>    #
>    # default implementation does nothing and returns self
> 
> The calls would look something like:
> 
> ob = ob._beforeMove(destination, destination.getPhysicalPath(), True)
> ... 
> moved_ob = moved_ob._afterMove(source, source.getPhysicalPath(), True)
> 
> etc
> 
> 
> Once we have a decent event system in place, these methods could be the
> place where copy / move events are generated.
> 
> Thoughts?

I'd prefer names less potentially prone to clashes with existing code, 
namely "_beforeItemMove", "_afterItemCopy", etc.

Also I tried to make a point on IRC about the recursion, and how we can 
switch this model to an event system later, but I probably wasn't clear 
enough. I'll try to explain better, but please bear with me :)


First and foremost, I want to be able to switch from the current system 
where each class has to implement (or inherit) _beforeItemMove so that 
recursion applies, which would conceptually mean a base class or a mixin, to 
a system driven by events.

Very importantly, this means that it's not _beforeItemMove's job to do the 
recursion. Why? Because suppose I inherit from a base class that has 
something to do before the move, and I want to add behavior to that. I'll 
have to call the base class, let it do its recursion, and then do my 
behavior on the current object. But suppose I inherit from two base classes 
that both had a _beforeItemMove that did recursion ? Then the recursion has 
to be defined and done only once, or you'd have double recursions (which 
turns into an exponential complexity). Maybe in some kind of unique base 
class? That's a total mess, I don't want to have to add artificial base 
classes to my code when there's no need to. I want the recursing behavior to 
be defined in a component (à la Zope 3), not in a base class. So the first 
consequence of having a clean approach is that:

     It's not the hook's job to do the recursion.

So the recursion has to be done by "external" code, a component, that calls 
all the relevant objects.


Now, in the current Zope 2 system having a class with a manage_beforeDelete 
(for instance) that just does "pass" means that the recursion is stopped. We 
want to keep that flexibility; this is easily done by having the hook return 
True if it wants to recurse (or to stop recursion -- we have to find what's 
best).


Here's how an event system would work (using afterItemCopy as an example):

1. something decides to copy an object. A "beforeCopy" event is sent. Then 
the copy is done. The an "afterCopy" event is sent.

2. some code having the need to tidy up things after the copy has a 
subscriber subscribed to the "afterCopy" event. For instance, there could be 
a subscriber in CMFCatalogAware.py that indexes the new objects, calling 
indexObject on each. A subscriber implements a policy, in this case it would 
be "copied objects have to be indexed in their new place". Notice, it's not 
indexObject's role to do any recursion.

3. the subscriber may or may not want the recursion to apply. How it does 
that is up to its implementation.

I'm stressing point 3 here because if I copy 100.000 objects I don't want 
the default system to send 100.000 events. If I have an efficient way of 
doing my bookkeeping, I don't want the event system do screw things up 
behind my back. Note for instance that in CMF reindexObjectSecurity does not 
do recursion like manage_afterAdd does, because it has optimized ways to do 
its job without recursing using objectValues().

So I strongly believe the default event system should send a simple event on 
copy (and I think I'll have to fight to get my way in Zope 3...). Now, if 
something wants to recurse, it's free to do so, and maybe send more events. 
But I want the system to be able to work without that.

Using this system, there could be a default subscriber in OFS/CopySupport.py 
that calls _afterItemCopy on the toplevel object and then does the 
recursion. The *subscriber* does the recursion, not _afterItemCopy.

But we're not there yet (no events in Zope 2), so instead of sending an 
event that's caught by a subscriber that does a recursion, this can be done 
for now by calling directly the code equivalent to the subscriber. Later 
we'll move to the event system. (And it is my hope that we can deprecate 
manage_beforeDelete & al. too.)


So I propose:

def _beforeItemMove(dest_container, dest_path):
     """
     Called before an item is moved.

     dest_container may be None (since for subobjects, the
     destination will not have been created yet).

     dest_path is a tuple containing the physical path of
     the new container (which may not yet exist).

     Returns True if recursion should stop.
     """

def _afterItemMove(source_container, source_path):
     """
     Called afer an item has been moved.

     source_container may be None (since for subobjects, the
     source may have been moved elsewhere).

     source_path is a tuple containing the physical path of
     the old container (which may no longer exist).

     Returns True if recursion should stop.
     """

def _beforeItemCopy(dest_container, dest_path):
     """
     Called before an item is copied.

     dest_container may be None (since for subobjects, the
     destination will not have been created yet).

     dest_path is a tuple containing the physical path of
     the new container (which may not yet exist).

     Returns True if recursion should stop.
     """

def _afterItemCopy(source_object):
     """
     Called after an item has been copied.

     source_object is the original object.

     Returns True if recursion should stop.
     """


Note, I'm not too sure about the source_container and source_object 
parameters, they're quite expensive to maintain. I'd like to see an 
implementation to discuss this.


Florent


-- 
Florent Guillaume, Nuxeo (Paris, France)   CTO, Director of R&D
+33 1 40 33 71 59   http://nuxeo.com   fg at nuxeo.com