Merge pull request #391 from Pylons/fix-attr-access

fix inadvertent deprecation warnings triggered by using the toolbar

CHANGES.txt

@@ -4,6 +4,10 @@ unreleased
 - Remove dependency on setuptools / pkg_resources.
   See https://github.com/Pylons/pyramid_debugtoolbar/pull/390
 
+- Avoid triggering DeprecationWarnings when tracking values for
+  deprecated attributes in Pyramid like ``effective_principals``.
+  See https://github.com/Pylons/pyramid_debugtoolbar/pull/391
+
 4.11 (2024-01-27)
 -----------------
 

src/pyramid_debugtoolbar/__init__.py

@@ -153,27 +153,26 @@ def _apply_parent_actions(parent_registry):
 
     toolbar_registry = toolbar_app.registry
 
-    # inject the BeforeRender subscriber after the application is created
-    # and all other subscribers are registered in hopes that this will be
-    # the last subscriber in the chain and will be able to see the effects
-    # of all previous subscribers on the event
     parent_config = Configurator(registry=parent_registry, introspection=False)
-
-    parent_config.add_subscriber(
-        'pyramid_debugtoolbar.toolbar.beforerender_subscriber',
-        'pyramid.events.BeforeRender',
-    )
-
     actions = toolbar_registry.queryUtility(IParentActions, default=[])
     for action in actions:
         action(parent_config)
+        parent_config.commit()
 
     # overwrite actions after they have been applied to avoid applying them
     # twice - but leave it as a new list incase someone adds more actions later
     # and calls config.make_wsgi_app() again... this would mainly be necessary
     # for tests that call config.make_wsgi_app() multiple times.
     toolbar_registry.registerUtility([], IParentActions)
 
+    # inject the BeforeRender subscriber after the application is created
+    # and all other subscribers are registered in hopes that this will be
+    # the last subscriber in the chain and will be able to see the effects
+    # of all previous subscribers on the event
+    parent_config.add_subscriber(
+        'pyramid_debugtoolbar.toolbar.beforerender_subscriber',
+        'pyramid.events.BeforeRender',
+    )
     parent_config.commit()
 
 

src/pyramid_debugtoolbar/panels/request_vars.py

@@ -1,10 +1,12 @@
 from pprint import saferepr
 
 from pyramid_debugtoolbar.panels import DebugPanel
-from pyramid_debugtoolbar.utils import dictrepr, wrap_load
+from pyramid_debugtoolbar.utils import dictrepr, patch_attrs
 
 _ = lambda x: x
 
+_marker = object()
+
 # extractable_request_attributes allow us to programmatically pull data
 # the format is (attr, is_dict)
 extractable_request_attributes = (
@@ -44,23 +46,31 @@
 #        For example `request.current_url()` is essentially `request.url`
 
 
-def extract_request_attributes(request):
+def extract_request_attributes(request, accessed_attrs):
     """
     Extracts useful request attributes from the ``request`` object into a dict.
     Data is serialized into a dict so that the original request is no longer
     needed.
     """
     extracted_attributes = {}
     for attr_, is_dict in extractable_request_attributes:
+        value = getattr(request, attr_, _marker)
         # earlier versions of pyramid may not have newer attrs
         # (ie, authenticated_userid)
-        if not hasattr(request, attr_):
+        if value is _marker:
+            continue
+        if is_dict and value:
+            value = value.__dict__
+        extracted_attributes[attr_] = value
+
+    for attr_, is_dict in lazy_request_attributes:
+        if attr_ not in accessed_attrs:
+            continue
+        value = getattr(request, attr_, _marker)
+        if value is _marker:
             continue
-        value = None
-        if is_dict and getattr(request, attr_):
-            value = getattr(request, attr_).__dict__
-        else:
-            value = getattr(request, attr_)
+        if is_dict and value:
+            value = value.__dict__
         extracted_attributes[attr_] = value
     return extracted_attributes
 
@@ -80,33 +90,19 @@ class RequestVarsDebugPanel(DebugPanel):
     def __init__(self, request):
         self.request = request
         self.data = data = {}
+        self.accessed_attrs = set()
         attrs = request.__dict__.copy()
         # environ is displayed separately
         del attrs['environ']
 
         if 'response' in attrs:
             attrs['response'] = repr(attrs['response'])
 
-        if 'session' in attrs:
-            self.process_session_attr(attrs['session'])
-            del attrs['session']
-        else:
-            # only process the session if it's accessed
-            wrap_load(
-                request,
-                'session',
-                self.process_session_attr,
-                reify=True,
-            )
-
-        for attr_, is_dict in lazy_request_attributes:
-            wrap_load(
-                request,
-                attr_,
-                lambda v, attr_=attr_, is_dict=is_dict: self.process_lazy_attr(
-                    attr_, is_dict, v
-                ),
-            )
+        install_attribute_listener(request, self.track_attribute_access)
+        # check if it's been potentially accessed already
+        for attr_, _ in lazy_request_attributes:
+            if attr_ in attrs:
+                self.accessed_attrs.add(attr_)
 
         # safely displaying the POST information is a bit tedious
         post_variables = None
@@ -145,33 +141,32 @@ def __init__(self, request):
                 'environ': dictrepr(request.environ),
                 'extracted_attributes': {},
                 'attrs': dictrepr(attrs),
-                'session': None,
             }
         )
 
-    def process_session_attr(self, session):
-        self.data.update(
-            {
-                'session': dictrepr(session),
-            }
-        )
-        return session
-
-    def process_lazy_attr(self, attr, is_dict, val_):
-        if is_dict:
-            val = val_.__dict__
-        else:
-            val = val_
-        self.data['extracted_attributes'][attr] = val
-        return val_
-
     def process_response(self, response):
-        extracted_attributes = extract_request_attributes(self.request)
+        extracted_attributes = extract_request_attributes(
+            self.request, self.accessed_attrs
+        )
         self.data['extracted_attributes'].update(extracted_attributes)
 
         # stop hanging onto the request after the response is processed
         del self.request
 
+    def track_attribute_access(self, item, value):
+        self.accessed_attrs.add(item)
+
+
+def install_attribute_listener(target, cb):
+    orig_getattribute = target.__class__.__getattribute__
+
+    def patched_getattribute(self, item):
+        value = orig_getattribute(self, item)
+        cb(item, value)
+        return value
+
+    patch_attrs(target, {'__getattribute__': patched_getattribute})
+
 
 def includeme(config):
     config.add_debugtoolbar_panel(RequestVarsDebugPanel)

src/pyramid_debugtoolbar/utils.py

@@ -24,6 +24,8 @@
 ROOT_ROUTE_NAME = 'debugtoolbar.root'
 EXC_ROUTE_NAME = 'debugtoolbar.exception'
 
+_marker = object()
+
 
 class ToolbarStorage(deque):
     """Deque for storing Toolbar objects."""
@@ -266,3 +268,34 @@ def wrapper(self):
         return cb(val)
 
     obj.set_property(wrapper, name=name, reify=reify)
+
+
+# copied from pyramid.utils.InstancePropertyHelper but without any support
+# for extra wrapping in properties, we want to keep that logic out of here
+# so we can do crazy things like define magic methods (__getattribute__)
+def patch_attrs(target, attrs):
+    parent = target.__class__
+    # fix the module name so it appears to still be the parent
+    # e.g. pyramid.request instead of pyramid.util
+    attrs.setdefault('__module__', parent.__module__)
+    newcls = type(parent.__name__, (parent, object), attrs)
+    # We assign __provides__ and __implemented__ below to prevent a
+    # memory leak that results from from the usage of this instance's
+    # eventual use in an adapter lookup.  Adapter lookup results in
+    # ``zope.interface.implementedBy`` being called with the
+    # newly-created class as an argument.  Because the newly-created
+    # class has no interface specification data of its own, lookup
+    # causes new ClassProvides and Implements instances related to our
+    # just-generated class to be created and set into the newly-created
+    # class' __dict__.  We don't want these instances to be created; we
+    # want this new class to behave exactly like it is the parent class
+    # instead.  See Pyramid GitHub issues #1212, #1529 and #1568 for more
+    # information.
+    for name in ('__implemented__', '__provides__'):
+        # we assign these attributes conditionally to make it possible
+        # to test this class in isolation without having any interfaces
+        # attached to it
+        val = getattr(parent, name, _marker)
+        if val is not _marker:
+            setattr(newcls, name, val)
+    target.__class__ = newcls