[ZCM] [ZC] 878/ 3 Comment "OFS/PropertyManager.py can incorrectly assume a plain dict is a REQUEST object"

Collector: Zope Bugs, Features, and Patches ... zope-coders-admin@zope.org
Fri, 11 Apr 2003 02:14:30 -0400


Issue #878 Update (Comment) "OFS/PropertyManager.py can incorrectly assume a plain dict is a REQUEST object"
 Status Pending, Zope/bug medium
To followup, visit:
  http://collector.zope.org/Zope/878

==============================================================
= Comment - Entry #3 by ctheune on Apr 11, 2003 2:14 am

For the sake of consistency we should not tell people to 
provide a dictionary instead of a request. Passing a dictionary
should be done by using **dict notation. 

Reason: There are a *lot* of places where REQUEST is supposed to
be a Request object and remains unchecked. I know there are some
fewer places where the documentation allows to pass a dictionary
as the Request object but look as if they could break in the
same way.
________________________________________
= Comment - Entry #2 by neaj on Apr 9, 2003 1:28 pm

It would definately be ugly if manage_changeProperties in
Zope-2.6.1-src/lib/python/OFS/PropertyManager.py
behaved differently from the one in
Zope-2.6.1-src/lib/python/OFS/PropertySheets.py

So either both have to get the more restrictive docstring, or
(preferably) both should cope with a plain dict as parameter. 
________________________________________
= Request - Entry #1 by neaj on Apr 9, 2003 1:20 pm


Uploaded:  "PropertyManager.py.patch"
 - http://collector.zope.org/Zope/878/PropertyManager.py.patch/view
I've encountered a very simple problem with
manage_changeProperties as defined in
Zope-2.6.1-src/lib/python/OFS/PropertyManager.py

The signature is:

    def manage_changeProperties(self, REQUEST=None, **kw):

and the docstring promises:

    Change object properties by passing either a mapping object
    of name:value pairs {'foo':6} or passing name=value parameters

However, if you call something.manage_changeProperties({a:b}),
then REQUEST will be {a:b} and the final check in
manage_changeProperties will be true, and manage_propertiesForm
will be returned:

    if REQUEST:
        message="Saved changes."
        return self.manage_propertiesForm(self,REQUEST,
                                  manage_tabs_message=message)

Now, manage_propertiesForm expects REQUEST to be a proper REQUEST
object and fails with a KeyError on URL1, when it is a simple
dictionary.

manage_changeProperties as defined in
Zope-2.6.1-src/lib/python/OFS/PropertySheets.py
doesn't suffer this problem, since it returns MessageDialog,
which doesn't expect anything beyond the keyword parameters it's
passed.

Either Zope-2.6.1-src/lib/python/OFS/PropertyManager.py should
be (1) taught to distinguish between {a:b} and REQUEST, or (2) the
docstring should be changed to:

    Change object properties by passing either the REQUEST,
    or passing name=value parameters 

I include a simplistic patch to do (1). 
==============================================================