[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