importToolset fails when trying to create a tool which subclasses ImmutableId with a non-default id

[David Glick]

Revision 115137 updated the importToolset method to no longer ignore errors when calling _setId on the just-created tool. This causes a problem when the following three conditions are true:
- the tool being created has an __init__ method with no required parameters (i.e. has a default id)
- the tool extends Products.CMFCore.utils.ImmutableId (i.e. disallows changes to its id)
- the XML being imported specifies a different id than the default for the tool

This is currently preventing use of the most recent GenericSetup release with Plone 4, since the package CMFEditions relies on being able to create a Products.CMFUid.UniqueIdHandlerTool.UniqueIdHandlerTool (a subclass of ImmutableId with a default id) with a non-default id.

[yuppie (yuppie3)]

This smells like a bug in CMFEditions, not in GenericSetup.

- How does CMFEditions set up a tool with the correct id if _setId fails?

- Why contains toolset.xml two tools with the same id (portal_uidhandler)?
http://svn.plone.org/svn/collective/Products.CMFEditions/trunk/Products/CMFEditions/profiles/default/toolset.xml

[David Glick (davisagli)]

Ah, it looks like before, the tool was created, the _setId call failed but that was silently ignored (so the tool has the wrong id), but the tool was still assigned to its container with the correct id. That does seem non-ideal, but it did allow the creation of two of these tools with different ids, and worked fine in practice. Do you have a suggestion of a better way to allow creating two instances of UniqueIdHandlerTool with different ids?

The fact that toolset.xml includes two copies of the portal_uidhandler tool looks like a silly mistake, but one that is irrelevant to this issue. The UniqueIdHandlerTool that is failing to install is the one that is supposed to get a different id, 'portal_historyidhandler'.

[yuppie (yuppie3)]

I guess I would use a different class with the id 'portal_historyidhandler'.

The strange thing is: CMFEditions *has* a different class with the id 'portal_historyidhandler':
Products.CMFEditions.UniqueIdHandlerTool.UniqueIdHandlerTool

I don't know how CMFEditions works, but

- the broken 'portal_uidhandler' definition in toolset.xml specifies that class

- adding 'portal_historyidhandler' TTW uses that class

So sorry for asking again: Are you really really sure you want to use Products.CMFUid.UniqueIdHandlerTool.UniqueIdHandlerTool and not Products.CMFEditions.UniqueIdHandlerTool.UniqueIdHandlerTool?

[David Glick (davisagli)]

I don't really know how CMFEditions is supposed to work either -- I'll try to take a closer look soon. The Products.CMFEditions.UniqueIdHandlerTool.UniqueIdHandlerTool implementation doesn't make much sense to me though...all its methods try to delegate to a tool with the id 'portal_historyidhandler' (its own id!)

[yuppie (yuppie3)]

This has piqued my curiosity...

Here are my results:

- I did look at the wrong toolset.xml. The trunk version seems to be broken, but is still unreleased.

- CMFEditions adds Products.CMFEditions.UniqueIdHandlerTool.UniqueIdHandlerTool (with id 'portal_historyidhandler') as 'portal_uidhandler' tool and Products.CMFUid.UniqueIdHandlerTool.UniqueIdHandlerTool (with id 'portal_uidhandler') as 'portal_historyidhandler' tool. Nasty but true.

- Products.CMFEditions.UniqueIdHandlerTool.UniqueIdHandlerTool is just some kind of proxy that allows to use 'portal_uidhandler' as an alias of 'portal_historyidhandler'.

- alecm tried to interchange the tool classes (r41276), hannosch reverted the cleanup (r44549+r44550) instead of fixing the BBB issues.

- The original install code was even more complex, the current profile is just a quick hack. See the ToDo in CHANGES.txt.

If nobody wants to clean up that mess in CMFEditions, I could live with this "solution" in GenericSetup:

- catch ValueErrors, log a warning and keep going

- add a note that this is necessary for providing BBB for CMFEditions hacks

[Godefroid Chapelle (gotcha)]

I dunno if I really cleaned up the mess in CMFEditions.

But I fixed this issue. IOW, you can close this bug.

[yuppie (yuppie3)]

@Godefroid: At least you cleaned up part of the mess in CMFEditions.

If you would have removed this broken and overridden element in toolset.xml it would be more obvious that Products.CMFEditions.UniqueIdHandlerTool.UniqueIdHandlerTool is unused dead code:

 <required tool_id="portal_uidhandler"
           class="Products.CMFEditions.UniqueIdHandlerTool.UniqueIdHandlerTool"/>

But that's off-topic here and not my business.

[Godefroid Chapelle (gotcha)]

I was waiting for comments from original authors before full cleanup.
Once I got those comments, I could finalize cleanup.

 <required tool_id="portal_uidhandler"
           class="Products.CMFEditions.UniqueIdHandlerTool.UniqueIdHandlerTool"/>

is gone.