Work around Products.CMFCore and Products.CMFPlone design bug

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 <https://github.com/zopefoundation/Zope/issues/1195>`_.
 
+- Work around ``Products.CMFCore`` and ``Products.CMFPlone`` design bug
+  (registering non callable constructors).
+  For details, see
+  `#1202 <https://github.com/zopefoundation/Zope/issues/1202>`_.
+
 
 5.9 (2023-11-24)
 ----------------

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):

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))