zopefoundation / Products.PluggableAuthService

Pluggable Zope authentication / authorization framework
Other
9 stars 18 forks source link

Error when importing ZODB User Manager (zexp) #95

Closed georgpfolz closed 3 years ago

georgpfolz commented 3 years ago

BUG/PROBLEM REPORT (OR OTHER COMMON ISSUE)

Importing a ZODB User Manager from zexp throws an error.

What I did:

I tried to transfer a ZODB User Manager to another Zope instance using Zope's import/export functionality (zexp files). When I try to import, I get the following error:

Traceback (innermost last):

Module ZPublisher.WSGIPublisher, line 167, in transaction_pubevents
Module ZPublisher.WSGIPublisher, line 376, in publish_module
Module ZPublisher.WSGIPublisher, line 271, in publish
Module ZPublisher.mapply, line 85, in mapply
Module ZPublisher.WSGIPublisher, line 68, in call_object
Module OFS.ObjectManager, line 653, in manage_importObject
Module OFS.ObjectManager, line 686, in _importObjectFromFile
TypeError: _setObject() got an unexpected keyword argument 'suppress_events'

what's the problem?

First I thought that this was weird, as ObjectManager's _setObject method has a keyword argument 'suppress_events'. But the overriding _setObject method of PluggableAuthService does not have this argument.

In ObjectManager._importObjectFromFile, the argument is passed to _setObject in any case, regardless of whether it's True or False.

So I guess, giving _setObject this keyword argument, even if it's never used, would enable the zexp import.

What version of Python and Zope/Addons I am using:

georgpfolz commented 3 years ago

I just made a monkey patch in PluggableAuthService.py:

def _setObject(self, id, object, roles=None, user=None, set_owner=0, suppress_events=None):
        #
        #   Override ObjectManager's version to change the default for
        #   'set_owner' (we don't want to enforce ownership on contained
        #   objects).
        Folder._setObject(self, id, object, roles, user, set_owner)

works!

icemac commented 3 years ago

The method signature of OFS.ObjectManager._setObject contains the suppress_events parameter, see https://github.com/zopefoundation/Zope/blob/25119a5ac7f783b7f59cd3dcaf2519676abeee65/src/OFS/ObjectManager.py#L327-L328

I think it should be added here, too, as you did in your patch. Additionally it even should get passed down to Folder._setObject.

A PR would be welcome.