[Framework-Team] Re: PLIP125 (link integrity) part II - redirect-on-move ready for merge
Hi, Martin Aspeli wrote: Hi guys, I've spent the past two days piecing together RedirectionTool and Whit's topp.rose product into something that meets the second half of PLIP125. Yeah, you rock as you always do! The first half is what Andreas is working on - warning when you're about to delete things that will break links. The second half is about offering automatic redirects or, if not possible, useful guesses instead of a dumb 404 page when an object has been moved or renamed. For the record, I love this functionality :) While I like this too, I'm a bit concerned about the performance penalty. Is there an easy way to turn this behavior off? Like a switch in some control panel? I'd like to merge this now, unless people have objections. There's no bundle, but the code is here: http://dev.plone.org/plone/browser/plone.app.redirector/trunk http://dev.plone.org/plone/browser/CMFPlone/branches/plip125-redirector The only thing that seems a bit dubious to me is the IGNORE_IDS constant in browser.py. I think this is a policy decision that should be easier to customize. The simplest solution here would be to turn this into a simple global utility with just one method that would return this list. This way people who really wanted to customize it could add their own local utility to return something different (for instance ignore all aliases registered on any content type + all dynamic view templates + ...). Also, unlike RedirectionTool, there is no form for making your own redirects (mainly because there's no good selection widget for me to use). This would work just fine (add things to the IRedirectionStorage utility), but since we have automatic redirects for anything moved or renamed, I think it's much less useful... in any case, it can be added later. Is there any way to clean up some old redirects, like a 'delete all redirects' option somewhere? I imagine that people might need this after a while. I'd like to merge before trunk moves too far away from my branch. :) +1 in general Hanno ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
[Framework-Team] Re: PLIP125 (link integrity) part II - redirect-on-move ready for merge
Hi Hanno, For the record, I love this functionality :) While I like this too, I'm a bit concerned about the performance penalty. Is there an easy way to turn this behavior off? Like a switch in some control panel? There is not, but there easily could be. The hardest part is making the control panel page. :) I don't think it's that much of a hit, though. On every object move or delete, the following operations take place: http://svn.plone.org/svn/plone/plone.app.redirector/trunk/plone/app/redirector/subscribers.py They basically look up and perform an operation on the storage utility in: http://svn.plone.org/svn/plone/plone.app.redirector/trunk/plone/app/redirector/storage.py Those operations are string manipulation and BTree lookups/inserts. I doubt they would cost much relative to the cost of renaming, cut/pasting or deleting an object in general. I'd be tempted to wait to make a switch until we have some evidence that it's a problem, but having those two event handlers short-circuit on some switch somewhere would be trivial. The other performance hit (which is bigger) is on the default_error_message page when it gets a NotFound error. In that case, we attempt to do a redirect (more string and btree logic), and possibly perform some traversal and catalog searches, in the first three methods in this view: http://svn.plone.org/svn/plone/plone.app.redirector/trunk/plone/app/redirector/browser.py I don't see why the 404 page needs to be fast, though (except during the spam attack we had on plone.org, but that's rather different). I'd like to merge this now, unless people have objections. There's no bundle, but the code is here: http://dev.plone.org/plone/browser/plone.app.redirector/trunk http://dev.plone.org/plone/browser/CMFPlone/branches/plip125-redirector The only thing that seems a bit dubious to me is the IGNORE_IDS constant in browser.py. I think this is a policy decision that should be easier to customize. The simplest solution here would be to turn this into a simple global utility with just one method that would return this list. This way people who really wanted to customize it could add their own local utility to return something different (for instance ignore all aliases registered on any content type + all dynamic view templates + ...). Good idea. I'd forgotten about that whart, actually, it was originally taken from the (age-old) RedirectionTool. Is there any way to clean up some old redirects, like a 'delete all redirects' option somewhere? I imagine that people might need this after a while. Not for now. The operations are there, it just requires some UI. I'm not 100% convinced its needed, but I agree that it feels like something that may need it one day (note that redirects are cleaned up when objects are deleted, so in theory only redirects to actual, live objects should exist, and of course, redirects are only created when an object is actually renamed or cut/pasted (moved), which in itself is not something you'd expect to happen with extreme frequency). Martin ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
[Framework-Team] Re: PLIP125 (link integrity) part II - redirect-on-move ready for merge
Hanno Schlichting wrote: The only thing that seems a bit dubious to me is the IGNORE_IDS constant in browser.py. I think this is a policy decision that should be easier to customize. The simplest solution here would be to turn this into a simple global utility with just one method that would return this list. This way people who really wanted to customize it could add their own local utility to return something different (for instance ignore all aliases registered on any content type + all dynamic view templates + ...). Fixed now - I went for an adapter instead, since conceivably you may want to vary it based on context (though context here is of course not the actual redirection target, and most often it'll be the portal root; the interface is registered for * by default). Martin ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
[Framework-Team] Re: PLIP125 (link integrity) part II - redirect-on-move ready for merge
Hi Hanno, Control panel pages that don't require any fancy widgets are really easy thx to formlib. If you look at my examples in plone.app.controlpanel, you will notice that they mostly consist of lots of boilerplate code one can copy'n'paste ;) Good point, thanks :) I don't think it's that much of a hit, though. On every object move or delete, the following operations take place: I'm not concerned about those. Right, agree. I'm mostly concerned about search engine crawlers revisiting already indexed sites. Not sure how much traffic those generate these days, though. Other than that the usual attacks websites have to endure come to my mind, like bots looking for insecure PHP apps on your domain (like awstats, phpmyadmin, ...). But probably you should filter those out in Apache or Squid in front of Plone anyways. Good catch - didn't think about that. I don't think a config option is a priority, but rather a nice-to-have feature :) I'd say we can merge before such an option is in place, definitely. Also, if you really need to turn it off, you have just to customise default_error_page.pt, though I agree that a switch would be nice (and not that hard). Martin ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
