ZCatalog manage_beforeDelete bug

[Martijn Faassen]

Almost there, a few points:

- If events are registered for base classes, of course they get triggered for
subclasses too. This had the interesting side effect that publications would
never show up in the SMI, because the 'added' event for both SilvaObject and
Folder called add_ordered_id, and that method is implemented in a particularly
stupid way: it calls refresh_ordered_ids directly, which adds the id if it's not
there, and removes it when it is. Hence, calling add_ordered_id twice will
result in removing the id again. I'll fix that.

- A tougher problem is that unindex_object() no longer seems to work for Silva
Root objects. I think the implementation is in zope itself, so I'll have to look
there for clues. This is what's causing some tests to fail.

[Eric Casteleijn (thisfred)]

corollary of the first point:

some code duplication could be removed, so we have a few less events now.

[Eric Casteleijn (thisfred)]

Something weird is going on: the uncataloging is triggered twice for all
objects. I suspect this may have been the case before we started refactoring, as
I remember seeing error messages to that effect.

[Eric Casteleijn (thisfred)]

This is turning out to be a major pain. After coming to terms with the seriously
weird event interface hierarchy (come on, added and removed are *NOT* kinds of
moved) we get into trouble with the catalog:

when versioned content objects are added or removed, we want to (un)catalog the
indexable versions. When moving a folder, the moved handler for versioned
content, gets triggered by a moved folder event, not a moved versioned content
event which we were expecting. We only have access to the old name and the old
parent of the *folder* that is being moved, not of the things in it. This means
we don't have the before state of the versioned content anymore, and no way to
recreate it. So we *will* have to split everything into WillBeMoved and Moved
handlers, so that we can call getPhysicalPath on the actual versioned content
object before it is moved.

Dunno if I'll finish that today, so this is as much a note to self as anything else.

[Daniel Nouri (daniel.nouri)]

This issue is not called 1*666* for no reason. Anyway, I fixed this in r23177:

In the 'versionedcontent_moved' handler we were making the wrong
assumption that event.object == object while in fact, event.object is
the original moved object and object might be any contained object as
well as the original object. We calculate the path to the object
accordingly.

We should probably still use ObjectWillBeMoved and friends, but right now I'm
happy that the tests are working again.

[Eric Casteleijn (thisfred)]

Done. Silva, SilvaDocument and SilvaMetadata all use events now, and show no
more deprecation warnings about manage_before* or manage_after* methods.

[Andy Altepeter (aaltepet)]

I've identified the bug with the second issue, double-calling of unindex_object. First, the problem:
When deleting through the SMI a container structure like:
/f
/f/f1

There will be an error like:
2008-02-29 22:29:20 ERROR Zope.ZCatalog uncatalogObject unsuccessfully attempted to uncatalog an object with a uid of /silva/f/f1.

Using a trial version of the WingIDE, I debugged this (which was sweet!), and found out that with the Zope 3-based events, lib/python/OFS/subscribers.py function dispathObjectWillBeMoved event recursively iterates down the to-be-removed (in this case removed) objects items and runs the events subscribed for each sub-item. Once that is finished, it calls the items manage_beforeDelete subscriber.

The objects manage_beforeDelete method, in Products.ZCatalog.CatalogPathAwareness.CatalogAware.manage_beforeDelete, first unindexes the item (in our example, '/f'), then iterates over all children, calling their manage_beforeDelete methods.

So really, now that we're in Zope3 event land, this CatalogAware.manage_beforeDelete doesn't need to iterate over children. Commenting out the children iterator loop fixes this error.

So now my next question: what are the next steps to getting this bug fixed in Zope? And most likely, the other manage_* methods in CatalogAware need to be similarly rewritten to remove the objectValues() loop.

[Andy Altepeter (aaltepet)]

added a monkey patch in monkey.py to work around the ZCatalog CatalogPath bug.

[Colin Watson (cjwatson)]

The zope2 project on Launchpad has been archived at the request of the Zope developers (see https://answers.launchpad.net/launchpad/+question/683589 and https://answers.launchpad.net/launchpad/+question/685285). If this bug is still relevant, please refile it at https://github.com/zopefoundation/zope2.