Submitted by : Bob McElrath at: 2005-02-04T11:00:31+00:00 (12 years ago)
Name :
Category : Severity : Status :
Optional subject :  
Optional comment :

initial proposal by bob mcelrath

Plugins should not use inheritance because:

  1. Depending on the order that plugins are loaded, they may need ZWiki to be reloaded.

    huron's comment To be more precise, they need ZWikiPage to be reloaded ... not the whole ZWiki ... so this is not so bad in terms of performance even if I can not prove that by any figures ...

  2. If two plugins force zwiki to be reloaded, the second reload will lose the first plugin.

    huron's comment After having thought about that, I dont think this is true any more. Every plugin makes a contribution to the PLUGINS array and only reloads ZWikiPage which uses PLUGINS. So, only ZWikiPage gets reloaded, not the PLUGINS array which gets updated by different plugins.

  3. Enumeration of plugins PLUGINS[1]? etc. require a fixed maximum number of plugins.

    huron's comment as it is sayed in __init__.py of plugins that << we can't modify __bases__ of an extension class - ZWikiPage.ZWikiPage >> ... see proposed solution later on this page

Furthermore, existing plugins (such as IssueTracker) are hard-wired into zwiki, and as such, aren't really plugins. In principle one should be able to remove a Tracker.py file and have your wiki still work.

I suggest a series of registerStuff methods (similar to registerPageType) which will store plugin information in member data of ZWikiPage. Each register method should take a callback method as an argument, and various places in the ZWiki code must be instrumented to call these callbacks. As a starting point I suggest:

  1. registerLinkType for new link types, such as issues. Callback methods should be kept in a list that will be looked at by ZWikiPage.renderLink.
  2. registerSkin called post-rendering to allow framing or skinning of the page. (i.e. adding IssueTracker header)
  3. registerMethod add a method available for pages that can be seen from Zope. (e.g. one can load a url such as FrontPage/mynewmethod) This can be done by directly modifying the dictionary for ZWikiPage. I put a small example at http://bob.mcelrath.org/dispatch.py

huron's comment before doing any real experiment, I think the problem with this approach is that no field can be registered (only methods) and for instance a rating plugin (which needs a "votes" dict to store the votes) could not work ... or the solution is to create a registerField method ... and this leads us to reinvent inheritance (i.e. the wheel)

Once all this is done all the plugins in the plugins/ directory should be reworked and then audited to make sure that if removed, zwiki still works. Once that is done, this bug should be closed, and we should all go have a beer.

Requests have also been made for wiki-syntax enhancement plugins, a la TWiki. This can be done straightforwardly with newstx, requiring two callbacks, one to identify the new syntax, and one to render it. LatexWiki, for instance, should be such a plugin.

second proposal by huron (based on bob mcelrath's)

I think we should use inheritance for basic behaviour i.e. new methods provided by the plugin ... because inheritance is exactly made for that and we will end up (with registerMethod stuff) doing it with our bare hands ...

But I completely agree with bob mcelrath's registerStuff for skin, link type and others.

Sadly, we cannot extends ZWikiPage at runtime because we cannot manipulate __bases__ of an extension class. So I tried to make ZWikiPage extends another class AllPlugin? whose __bases__ I manipulate at runtime (registering plugins) to add new classes. And it seems to work :

  1. No problem to register multiple plugins
  2. No need to reload anything since we manipulate the class AllPlugins? at runtime
  3. No fixed Enumeration of plugins (for the same reason)

debug session example (with two plugins in two separate products) :

  ./bin/zopectl debug
  Starting debugger (the name "app" is bound to the top-level Zope object)
  >>> w= app.tt.sssssssssssss.WikiPage200502048162875024
  >>> w
  <ZWikiPage at /tt/sssssssssssss/WikiPage200502048162875024>
  >>> w.sayHelloPlugin1()
  hello plugin 1
  >>> w.sayHelloPlugin2()
  hello plugin 2
  >>> w.__class__.__bases__
  (<class Products.ZWiki.Editing.EditingSupport at 0x41bf2c2c>, [...] <class Products.ZWiki.plugins.AllPlugins at 0x41b9968c>)
  >>> w.__class__.__bases__[13].__bases__
  (<class Products.ZWiki.plugins.PurpleNumbers.PurpleNumbersSupport at 0x41b995cc>, 
  <class Products.ZWiki.plugins.Tracker.TrackerSupport at 0x41ba195c>,
  <class Products.ZWiki.plugins.Rating.RatingSupport at 0x41ba1c2c>, 
  <class Products.ZWiki.plugins.Fit.FitSupport at 0x41bb444c>, 
  <class Products.ZWikiPlugin1.MyPlugin1.MyPlugin1Support at 0x41c00a4c>, 
  <class Products.ZWikiPlugin2.MyPlugin2.MyPlugin2Support at 0x72d90a5d>, 

here is the patch (very short) http://paste.plone.org/590

This seems promising ... but I might be missing something ... I'd appreciate any feedbacks before I consider this part is done and start to dig into the rest ...


comments:

... --Bob McElrath?, Mon, 07 Feb 2005 10:05:41 -0800 reply
Look again at my dispatch.py example. It is possible to add methods to ZWikiPage without inheritance. It is also possible to add data members in exactly the same way. (elements of __dict__ can be either data or methods) Your approach seems to work. However I'm still wary of using inheritance because plugins seem to me a has-a relationship rather than an is-a relationship. Has-a relationships should in general be members, not parents.

Any idea what will happen if a plugin is removed? With inheritance, a ZWikiPage becomes invalid. If the plugins were members, would the object still be valid if the plugin were removed or drastically rewritten? e.g. If someone tries the plugin Ratings, then decides they don't want it, is their wiki "infected" or can the data members added by Ratings be removed? Can the data members be ignored? Is there a difference in the behavior of inheritance/member data?

... huron -- Tue, 08 Feb 2005 04:48:10 -0800 reply
I understand your point about has-a relationship rather than an is-a relationship. But, my point is that adding methods and fields manipulating __dict__ you'll end up adding all your methods and fields (methods not added will be useless because not possible to call) ... and that is doing inheritance a la mano. That's why now I believe that inheritance should be the way to go.

In inheritance : if a plugin is removed, then ZWikiPage does not implement the methods any anymore (nothing invalid). Objects (ZWikiPage instances) created while the plugin was available keep the fields the plugin provided. I think there is no difference beetween the two approaches regarding object fields. But, for methods, the registerStuff approach is much worse since objects created during the time the plugin was available have additionnal methods (added to then by __init__.py like in you dispatch example) will still have the plugin methods. This is due to the fact that inheritance adds methods to classes and registerStuff adds methods to instances.

My plan :

  1. modify the plugin mechanism so that is uses clean inheritance and no fixed plugin slots (100% complete)
  2. create functions in UI & related modules so that my external test plugin can work with no dirty hack into ZWiki itself (100% complete)
  3. backport usage of thoses functions (and create new functions) to the tracker to make it a real plugin (50% complete)

Results :

here you can find the modified version of ZWiki. If tried to make the changes minimal and located in the plugin directory (only the last two patches are mine).

ZWiki: darcs get http://darcs.sopinspace.com/ZWiki/

A very simple external plugin

ZWikiSimplePlugin: darcs get http://darcs.sopinspace.com/ZWikiSimplePlugin/

A simple external plugin (very similar to the tracker) I only tested it within plone so far

ZWikiVote: darcs get http://darcs.sopinspace.com/ZWikiVote/

... any feedback most welcome ...

Still missing :

Refresh problem common --Martijn Pieters, Fri, 11 Mar 2005 05:08:34 -0800 reply
I wouldn't worry about the ZWiki refresh problem; it is a common problem with Product refreshing where Products with registries get cleared. There isn't a good solution to that other than manually marking all the products that register plugins as auto-refreshing and select them on the ZWiki Product refresh tab as dependents.

? -- huron -- Tue, 26 Apr 2005 05:24:12 -0700 reply
... but should be sufficient to get feeback on whether or not my solution seems good or not. (sorry for the delay, I've been very busy ... but I think this pluginisation is a long run issue anyway )

downloading --simon, Fri, 02 Sep 2005 12:40:30 -0700 reply
huron, I'm pulling from your repo so I can take a closer looker at what you did. Did you catch Bob's point about classes being broken.. my experience has been that you can install something like LatexWiki, create wiki page instances that have it as a mixin, then uninstall LatexWiki, and end up with broken objects as a result. I'm definitely keen to find a way around that.

Also, I think it's worth merging pagetypes/ into plugins/ somehow, for simplicity. Comments ?

tests --simon, Sat, 03 Sep 2005 10:49:32 -0700 reply
I tried renaming subclasses one level up and two levels up and couldn't reproduce that problem. Perhaps it's to do with being in a separate product.

updates --simon, Thu, 15 Sep 2005 18:22:05 -0700 reply
I've added a hook mechanism (http://joyful.com/darcsweb/darcsweb.cgi?r=ZWiki;a=commitdiff;h=20050916001646-e02d6-15ad1d173304aba29effa33d704b50113f792de8.gz), which is like register* methods but more flexible. It stores the hooks as module-level globals at init time so I hope there won't be an issue with old hooks being left hanging around.

I've instrumented the upgrade and upgradeId methods with this and removed the hard-coded tracker dependencies there. I listed some others in the plugins package docstring (http://joyful.com/repos/ZWiki/plugins/__init__.py).

Also I moved the plugins to the beginning of the inheritance list, so they can now override any of the core page methods as someone requested on another page. I hope this is reasonable, let me know if you think of problems.

I think the array slots kludge and huron's idea of one more subclass are pretty much equivalent as far as python is concerned, I haven't changed any more there yet.

updates --simon, Thu, 15 Sep 2005 18:24:52 -0700 reply
PS I decided to leave pagetypes separate for the moment.