[Framework-Team] [PLIP 125 Link Integrity] - Review notes, part I

Martin Aspeli optilude at gmx.net
Thu Sep 14 23:00:45 UTC 2006


Hi guys,

These notes are about PLIP125, Link Integrity. This PLIP contains two parts:

  - Warn users when they try to delete things that would break links
  - Warn users when they try to move things that would break links 
and/or update reference automatically and/or provide a redirect

These notes only cover part I. Part II is envisaged to be delivered via 
whit's topp.rose product, which Andi is currently looking into.

=====================
1. Getting the bundle
=====================

I used Zope 2.10 from svn.

svn co https://svn.plone.org/svn/plone/review/plip125-link-integrity-bundle

Start Zope, add a Plone site, and quick-install the LinkIntegrity product.

Things to try:

  - Add some content
  - Add a page which links to this content
  - Delete some of the dependent content (e.g. images referenced)

You will get a confirmation page (you may get two, actually - the first 
one comes from Plone, the second one from LinkIntegrity - they would 
probably need to be integrated). If you click "Yes" deletion takes 
place, otherwise it does not.

  *Note* - I'm getting an exception raised if I try to cancel the 
delete. I think this is something that's crept in very recently, and 
I've told Andi about it. This fix should be trivial, though - for now, 
just click on another link (e.g. the navtree) to prove that the object 
is not gone.

===================
2. How does it work
===================

Andi's solution is quite innovative and very generic. Basically, it 
works like this:

  - When an AT object is saved or modified it is scanned for text 
fields. These in turn are parsed for <a> and <img> tags that link to 
internal resources.

  - If you think about it, hyperlinks are just a special case of 
references. LinkIntegrity makes these explicit by registering them as 
references in AT's reference catalog. These are of a special reference 
class called NotifyingReference.

  - When a referenced object is deleted, AT has a hook called 
beforeTargetDeleteInformSource(). LinkIntegrity implements this to raise 
a ReferencedObjectRemovedEvent.

This is where it gets interesting. At this point, we essentially almost 
at the stage where the object will be _delObject()'d.

  - The handler for ReferencedObjectRemovedEvent raises a 
LinkIntegrityException, which is a subclass of 
OFS.ObjectManager.BeforeDeleteException. This special kind of exception 
is allowed by ObjectManager to be raised to abort deletion, essentially.

  - However, we don't want to just abort, we want to ask the user if 
they know what they're doing. Here, LinkIntegrity borrows from 
FiveException, by installing a wrapper around 
zpublisher_exception_hook(). This wrapper will attempt to look up a view 
(name='index.html') on the *exception* and the request. That way, we can 
register a view on the exception to display something more sensible than 
a traceback.

  - In this case, we display a confirmation page.

  - If the user clicks No, the whole deletion operation is aborted

  - If the user chooses to go ahead, we need to re-do the operation they 
originally requested. Since we can't know for sure what initiated the 
delete operation (and thus where the user expects to be taken to, what 
information should be passed along in the request etc), we re-run the 
original request. Zope actually supports this, through something called 
a Retry exception. By raising one of these, Zope will re-run the request 
(which is marshalled on the confirmation page so that we don't lose it). 
However, before re-raising it, a marker is set to ensure the 
confirmation page isn't displayed the second time. This results in the 
actual delete operation taking place.

There are a few things that feel a bit strange here, so I'll try to 
address them and why I've come around to thinking they're still OK :)

2.1. Why not just do this in folder_delete_confirm?

This solution is very generic. Having to do the checking and 
confirmation in a page template would be possible, but would require all 
such page templates to be adjusted. Consider the case where you delete a 
Ploneboard comment that someone has linked to from somewhere else in 
your portal using Ploneboard's own delete button, for example - this 
approach is very thorough in informing you of broken links (due to 
deletes, the move/rename use case is handled by the second part of the PLIP)

Of course, the keeping of references by parsing the saved/modified HTML 
would still be necessary even if this was done only at the template 
level - otherwise, we'd need to scan the entire portal for 
back-references on delete. :-)

2.2. The monkey patch to wrap zpublisher_before_exception_hook()

This is actually taken from FiveException, which introduced the idea. 
It's quite a useful concept to be able to use a particular view on an 
exception rather than just a traceback. It's conceivable that this may 
make its way into Zope eventually. The patch (in monkey.py) is fairly 
non-invasive - it just attempts the view lookup and if it fails, it goes 
back to calling the original hook.

2.3. AT references suck!

Indeed they do, but I believe their slowness is mostly felt when they 
are accessed, which only happens once, when the object is deleted.

2.4. What about non-Archetypes content?

A similar pattern could be employed. This could probably use an 
IObjectWillBeDeleted type event instead of the 
beforeTargetDeleteNotifySource() mechanism of AT references (a direct 
object event won't work here because of some legacy manage_beforeDelete 
majik, but this is probably only an issue on CMF/AT objects). It would 
also need an adapter to extract HTML to parse for links, since iterating 
over the Schema() won't work.

This should be fairly trivial. However, there's not much point until 
there is a generic reference engine implementation that we can rely on.

2.5. This all sounds really complicated

There's actually surprisingly little code here, and it's fairly 
straightforward to follow it. Andi has also provided very good, detailed 
notes on the implementation and why has to work the way it does. I don't 
think it poses a major maintenance risk. There is also good test 
coverage (and Andi had to fix a couple of bugs in the test framework in 
the process, which is the source of another monkey patch for now, until 
the test case code can be fixed properly).

===========================
3. Concerns and suggestions
===========================

  - If you have a reference A -> B, and no other references, you delete 
A and B together, you may still get a confirmation page about deleting 
B. That is troublesome, at least if you have a folder full of objects 
referencing each other, or you are trying to delete a page and its fifty 
images in one go.

I'm not actually sure what the best solution here is. I've deferred to 
Andi for some more thought. It may be possible to build more logic into 
the delete-confirm view to either inform or forgo the generic delete 
handler that raises the exception and exposes the confirm screen.

  - We need to ensure this doesn't interfere with DAV or FTP deletes

  - We need to verify that this doesn't impose too much of a performance 
overhead

  - The confirmation page needs a touch of visual love

  - There is already a "delete confirm" page when you select the Delete 
action from the actions drop-down (I believe this used to be a JS 
pop-up). It'd be nice of the delete protection could be applied here so 
that you don't potentially get two confirmation pages.

   - There may need to be a way to switch this behaviour off. Andi 
actually had that (of sorts) before, relying on a Zope 2 tool, which 
then lost its purpose and went away. A switch somewhere is easy to do, 
though.

==============================
4. Recommendations and caveats
==============================

I think this is really important functionality, and something which 
could differentiate Plone for people with active sites that are 
published publicly.

I would really like to see this go in, but I think we need to discuss 
the issue with interdependent deletes a bit more first.

If/when we merge, this could move to the plone.app space. That'd be 
purely aesthetics, though. The LinkIntegrity skin layer does contain a 
customised version of folder-delete to work around some over-zealous 
exception checking, but this is easily merged into Plone core. At this 
point, LI won't require a quick-install at all.

Martin





More information about the Framework-Team mailing list