From 22adba16dd54480b40ccbbd18c26f70204d5ba24 Mon Sep 17 00:00:00 2001 From: Dieter Maurer Date: Tue, 23 Apr 2024 19:56:48 +0200 Subject: [PATCH] Work around `Products.CMFCore` and `Products.CMFPlone` registering non callable constructors (#1205) --- CHANGES.rst | 5 + src/App/ProductContext.py | 40 +++++- src/App/tests/test_ProductContext.py | 190 +++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 6 deletions(-) create mode 100644 src/App/tests/test_ProductContext.py diff --git a/CHANGES.rst b/CHANGES.rst index 3b9767a59d..4bc40ca282 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -32,6 +32,11 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst - Fix authentication error viewing ZMI with a user defined outside of zope root. Fixes `#1195 `_. +- Work around ``Products.CMFCore`` and ``Products.CMFPlone`` design bug + (registering non callable constructors). + For details, see + `#1202 `_. + 5.9 (2023-11-24) ---------------- diff --git a/src/App/ProductContext.py b/src/App/ProductContext.py index d33a20034f..0a97b2b8ef 100644 --- a/src/App/ProductContext.py +++ b/src/App/ProductContext.py @@ -49,16 +49,13 @@ def registerClass(self, instance_class=None, meta_type='', permission=None, constructors=(), icon=None, permissions=None, legacy=(), visibility="Global", interfaces=_marker, - container_filter=None): + container_filter=None, resources=()): """Register a constructor Keyword arguments are used to provide meta data: instance_class -- The class of the object that will be created. - This is not currently used, but may be used in the future to - increase object mobility. - meta_type -- The kind of object being created This appears in add lists. If not specified, then the class meta_type will be used. @@ -71,7 +68,7 @@ def registerClass(self, instance_class=None, meta_type='', A method can be a callable object with a __name__ attribute giving the name the method should have in the product, or the method may be a tuple consisting of a - name and a callable object. The method must be picklable. + name and a callable object. The first method will be used as the initial method called when creating an object. @@ -96,6 +93,13 @@ class will be registered. and before pasting (after object copy or cut), but not before calling an object's constructor. + resources -- a sequence of resource specifications + A resource specification is either an object with + a __name__ attribute or a pair consisting of the resource + name and an object. + The resources are put into the ProductFactoryDispather's + namespace under the specified name. + """ pack = self.__pack initial = constructors[0] @@ -203,7 +207,31 @@ class __FactoryDispatcher__(FactoryDispatcher): else: name = os.path.split(method.__name__)[-1] if name not in productObject.__dict__: - m[name] = zpublish_wrap(method) + if not callable(method): + # This code is here because ``Products.CMFCore`` and + # ``Products.CMFPlone`` abuse the ``constructors`` + # parameter to register resources violating the explicit + # condition that constructors must be callable. + # It should go away once those components have been fixed. + from warnings import warn + warn("Constructors must be callable; " + "please use `resources` " + "(rather than `constructors`) to register " + "non callable objects", + DeprecationWarning, + 2) + m[name] = method + else: + m[name] = zpublish_wrap(method) + m[name + '__roles__'] = pr + + for resource in resources: + if isinstance(resource, tuple): + name, resource = resource + else: + name = resource.__name__ + if name not in productObject.__dict__: + m[name] = resource m[name + '__roles__'] = pr def getApplication(self): diff --git a/src/App/tests/test_ProductContext.py b/src/App/tests/test_ProductContext.py new file mode 100644 index 0000000000..7f349ac84d --- /dev/null +++ b/src/App/tests/test_ProductContext.py @@ -0,0 +1,190 @@ +############################################################################## +# +# Copyright (c) 2024 Zope Foundation and Contributors. +# +# This software is subject to the provisions of the Zope Public License, +# Version 2.1 (ZPL). A copy of the ZPL should accompany this distribution. +# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED +# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS +# FOR A PARTICULAR PURPOSE. +# +############################################################################## +"""Partial ``ProductContext`` tests. +""" + +from types import ModuleType +from types import SimpleNamespace +from unittest import TestCase + +from ZPublisher import zpublish_marked + +from ..ProductContext import ProductContext + + +class ProductContextTests(TestCase): + def setUp(self): + self.args = _Product(), _App(), ModuleType("pack") + self.pc = ProductContext(*self.args) + # ``ProductContext.registerClass`` has a lot of global + # side effects. We save the original values, set up values + # for our tests and restore the original values in ``teadDown`` + # When further tests are added, the list likely needs to + # be extended + self._saved = {} + for ospec, val in { + "sys.modules['Products']": ModuleType("Products"), + "AccessControl.Permission._registeredPermissions": {}, + "AccessControl.Permission._ac_permissions": (), + "AccessControl.Permission.ApplicationDefaultPermissions": + _ApplicationDefaultPermissions}.items(): + obj = _resolve(ospec) + self._saved[ospec] = obj.get() + obj.set(val) + + def tearDown(self): + for ospec, val in self._saved.items(): + _resolve(ospec).set(val) + + def test_initial_tuple(self): + + def c(): + pass + + self.pc.registerClass(meta_type="test", permission="test", + constructors=(("name", c),)) + self._verify_reg("name", "c") + + def test_initial_named(self): + + def c(): + pass + + self.pc.registerClass(meta_type="test", permission="test", + constructors=(c,)) + self._verify_reg("c", "c") + + def test_constructor_tuple(self): + + def initial(): + pass + + def c(): + pass + + self.pc.registerClass(meta_type="test", permission="test", + constructors=(initial, ("name", c),)) + self._verify_reg("name", "c") + + def test_constructor_named(self): + + def initial(): + pass + + def c(): + pass + + self.pc.registerClass(meta_type="test", permission="test", + constructors=(initial, c,)) + self._verify_reg("c", "c") + + def test_constructor_noncallable(self): + + def initial(): + pass + + nc = SimpleNamespace(__name__="nc") + with self.assertWarns(DeprecationWarning): + self.pc.registerClass(meta_type="test", permission="test", + constructors=(initial, nc)) + self._verify_reg("nc", nc) + + def test_resource_tuple(self): + + def initial(): + pass + + r = SimpleNamespace() + self.pc.registerClass(meta_type="test", permission="test", + constructors=(initial,), + resources=(("r", r),)) + self._verify_reg("r", r) + + def test_resource_named(self): + + def initial(): + pass + + r = SimpleNamespace(__name__="r") + self.pc.registerClass(meta_type="test", permission="test", + constructors=(initial,), + resources=(r,)) + self._verify_reg("r", r) + + def _verify_reg(self, name, obj): + pack = self.args[-1] + m = pack._m + fo = m[name] + if isinstance(obj, str): + self.assertEqual(obj, fo.__name__) + self.assertTrue(zpublish_marked(fo)) + else: + self.assertIs(obj, fo) + + +class _Product: + """Product mockup.""" + def __init__(self): + self.id = "pid" + + +class _App: + """Application mockup""" + + +class _ApplicationDefaultPermissions: + """ApplicationDefaultPermissions mockup.""" + + +class _Attr(SimpleNamespace): + """an attribute.""" + + def get(self): + return getattr(self.o, self.a) + + def set(self, val): + setattr(self.o, self.a, val) + + +class _Item(SimpleNamespace): + """a (mapping) item.""" + + def get(self): + # we use ``_Item`` for missiong + return self.o.get(self.a, _Item) + + def set(self, val): + # we use ``_Item`` for missiong + if val is _Item: + del self.o[self.a] + else: + self.o[self.a] = val + + +def _resolve(spec): + """resolve *spec* into an ``_Attr`` or ``_Item``. + + *spec* is a dotted name, optionally followed by a subscription. + """ + if "[" in spec: + dotted, sub = spec[:-1].split("[") + else: + dotted, sub = spec, None + o = None + segs = dotted.split(".") + for i in range(0, len(segs) - (0 if sub is not None else 1)): + seg = segs[i] + o = getattr(o, seg, None) + if o is None: + o = __import__(".".join(segs[:i + 1]), fromlist=(seg,)) + return _Attr(o=o, a=segs[-1]) if sub is None else _Item(o=o, a=eval(sub))