Add support for subclasses to implement _p_repr. Fixes #11

CHANGES.rst

@@ -4,6 +4,12 @@
 4.3.1 (unreleased)
 ------------------
 
+- Change the default representation of ``Persistent`` objects to
+  include the representation of their OID and jar, if set. Also add
+  the ability for subclasses to implement ``_p_repr()`` instead of
+  overriding ``__repr__`` for better exception handling. See `issue 11
+  <https://github.com/zopefoundation/persistent/issues/11>`_.
+
 - Reach and maintain 100% test coverage.
 
 - Simplify ``__init__.py``, including removal of an attempted legacy

docs/using.rst

-Using :mod:`persistent` in your application
-===========================================
+=============================================
+ Using :mod:`persistent` in your application
+=============================================
 
 
 Inheriting from :class:`persistent.Persistent`
-----------------------------------------------
+==============================================
 
 The basic mechanism for making your application's objects persistent
 is mix-in interitance.  Instances whose classes derive from
@@ -14,7 +15,7 @@ they have been changed.
 
 
 Relationship to a Data Manager and its Cache
---------------------------------------------
+============================================
 
 Except immediately after their creation, persistent objects are normally
 associated with a :term:`data manager` (also referred to as a :term:`jar`).
@@ -63,7 +64,7 @@ The examples below use a stub data manager class, and its stub cache class:
 
 
 Persistent objects without a Data Manager
------------------------------------------
+=========================================
 
 Before aersistent instance has been associtated with a a data manager (
 i.e., its ``_p_jar`` is still ``None``).
@@ -166,7 +167,7 @@ Try all sorts of different ways to change the object's state:
 
 
 Associating an Object with a Data Manager
------------------------------------------
+=========================================
 
 Once associated with a data manager, a persistent object's behavior changes:
 
@@ -219,7 +220,7 @@ control the state as described below, or use a
 :class:`~.PersistentList` or :class:`~.PersistentMapping`.
 
 Explicitly controlling ``_p_state``
------------------------------------
+===================================
 
 Persistent objects expose three methods for moving an object into and out
 of the "ghost" state::  :meth:`persistent.Persistent._p_activate`,
@@ -328,7 +329,7 @@ which is exactly the same as calling ``_p_activate``:
 
 
 The pickling protocol
----------------------
+=====================
 
 Because persistent objects need to control how they are pickled and
 unpickled, the :class:`persistent.Persistent` base class overrides
@@ -382,7 +383,7 @@ The ``_p_serial`` attribute is not affected by calling setstate.
 
 
 Estimated Object Size
----------------------
+=====================
 
 We can store a size estimation in ``_p_estimated_size``. Its default is 0.
 The size estimation can be used by a cache associated with the data manager
@@ -412,7 +413,7 @@ Of course, the estimated size must not be negative.
 
 
 Overriding the attribute protocol
----------------------------------
+=================================
 
 Subclasses which override the attribute-management methods provided by
 :class:`persistent.Persistent`, but must obey some constraints:
@@ -448,3 +449,24 @@ Subclasses which override the attribute-management methods provided by
 :meth:`__getattr__`
   For the ``__getattr__`` method, the behavior is like that for regular Python
   classes and for earlier versions of ZODB 3.
+
+
+Implementing ``_p_repr``
+========================
+
+Subclasses can implement ``_p_repr`` to provide a custom
+representation. If this method raises an exception, the default
+representation will be used. The benefit of implementing ``_p_repr``
+instead of overriding ``__repr__`` is that it provides safer handling
+for objects that can't be activated because their persistent data is
+missing or their jar is closed.
+
+.. doctest::
+
+   >>> class P(Persistent):
+   ...    def _p_repr(self):
+   ...        return "Custom repr"
+
+   >>> p = P()
+   >>> print(repr(p))
+   Custom repr

persistent/cPersistence.c

@@ -1373,28 +1373,40 @@ Per_set_sticky(cPersistentObject *self, PyObject* value)
 }
 
 static PyObject*
-repr_helper(PyObject *o, char* format)
+repr_format_exception(char* format)
 {
-    /* Returns a new reference, or NULL on error */
+    /* If an exception we should catch occurred, return a new
+       string of its repr. Otherwise, return NULL. */
     PyObject *exc_t;
     PyObject *exc_v;
     PyObject *exc_tb;
+    PyObject *result = NULL;
+
+    if (PyErr_Occurred() && PyErr_ExceptionMatches(PyExc_Exception))
+    {
+        PyErr_Fetch(&exc_t, &exc_v, &exc_tb);
+        PyErr_NormalizeException(&exc_t, &exc_v, &exc_tb);
+        PyErr_Clear();
+
+        result = PyUnicode_FromFormat(format, exc_v);
+        Py_DECREF(exc_t);
+        Py_DECREF(exc_v);
+        Py_DECREF(exc_tb);
+    }
+    return result;
+}
+
+static PyObject*
+repr_helper(PyObject *o, char* format)
+{
+    /* Returns a new reference, or NULL on error */
     PyObject *result;
 
     if (o)
     {
         result = PyUnicode_FromFormat(format, o);
-        if (!result && PyErr_Occurred() && PyErr_ExceptionMatches(PyExc_Exception))
-        {
-            PyErr_Fetch(&exc_t, &exc_v, &exc_tb);
-            PyErr_NormalizeException(&exc_t, &exc_v, &exc_tb);
-            PyErr_Clear();
-
-            result = PyUnicode_FromFormat(format, exc_v);
-            Py_DECREF(exc_t);
-            Py_DECREF(exc_v);
-            Py_DECREF(exc_tb);
-        }
+        if (!result)
+            result = repr_format_exception(format);
     }
     else
     {
@@ -1408,10 +1420,31 @@ repr_helper(PyObject *o, char* format)
 static PyObject*
 Per_repr(cPersistentObject *self)
 {
+    PyObject *prepr = NULL;
+    PyObject *prepr_exc_str = NULL;
+
     PyObject *oid_str = NULL;
     PyObject *jar_str = NULL;
     PyObject *result = NULL;
 
+    prepr = PyObject_GetAttrString((PyObject*)Py_TYPE(self), "_p_repr");
+    if (prepr)
+    {
+        result = PyObject_CallFunctionObjArgs(prepr, self, NULL);
+        if (result)
+            goto cleanup;
+        else
+        {
+            prepr_exc_str = repr_format_exception(" _p_repr %R");
+            if (!prepr_exc_str)
+                goto cleanup;
+        }
+    }
+    else
+    {
+        PyErr_Clear();
+        prepr_exc_str = PyUnicode_FromString("");
+    }
 
     oid_str = repr_helper(self->oid, " oid %R");
     if (!oid_str)
@@ -1421,11 +1454,13 @@ Per_repr(cPersistentObject *self)
     if (!jar_str)
         goto cleanup;
 
-    result = PyUnicode_FromFormat("<%s object at %p%S%S>",
+    result = PyUnicode_FromFormat("<%s object at %p%S%S%S>",
                                   Py_TYPE(self)->tp_name, self,
-                                  oid_str, jar_str);
+                                  oid_str, jar_str, prepr_exc_str);
 
 cleanup:
+    Py_XDECREF(prepr);
+    Py_XDECREF(prepr_exc_str);
     Py_XDECREF(oid_str);
     Py_XDECREF(jar_str);
 

persistent/interfaces.py

@@ -166,6 +166,14 @@ class IPersistent(Interface):
     these objects are invalidated, they immediately reload their state
     from their data manager, and are then in the saved state.
 
+    reprs
+
+    By default, persistent objects include the reprs of their
+    _p_oid and _p_jar, if any, in their repr. If a subclass implements
+    the optional method ``_p_repr``, it will be called and its results returned
+    instead of the default repr; if this method raises an exception, that
+    exception will be caught and its repr included in the default repr.
+
     """
 
     _p_jar = Attribute(
@@ -314,10 +322,10 @@ class IPersistent(Interface):
 
     def _p_getattr(name):
         """Test whether the base class must handle the name
-   
+
         The method unghostifies the object, if necessary.
         The method records the object access, if necessary.
- 
+
         This method should be called by subclass __getattribute__
         implementations before doing anything else. If the method
         returns True, then __getattribute__ implementations must delegate
@@ -471,7 +479,7 @@ class IPickleCache(Interface):
         """ Perform an incremental garbage collection sweep.
 
         o Reduce number of non-ghosts to 'cache_size', if possible.
-        
+
         o Ghostify in LRU order.
 
         o Skip dirty or sticky objects.
@@ -505,7 +513,7 @@ class IPickleCache(Interface):
 
         If the object's '_p_jar' is not None, raise.
 
-        If 'oid' is already in the cache, raise. 
+        If 'oid' is already in the cache, raise.
         """
 
     def reify(to_reify):
@@ -536,7 +544,7 @@ class IPickleCache(Interface):
         o Any OID corresponding to a p-class will cause the corresponding
             p-class to be removed from the cache.
 
-        o For all other OIDs, ghostify the corrsponding object and 
+        o For all other OIDs, ghostify the corrsponding object and
             remove it from the ring.
         """
 

persistent/persistence.py

@@ -558,6 +558,14 @@ class Persistent(object):
             return cache.get(oid) is self
 
     def __repr__(self):
+        p_repr_str = ''
+        p_repr = getattr(type(self), '_p_repr', None)
+        if p_repr is not None:
+            try:
+                return p_repr(self)
+            except Exception as e:
+                p_repr_str = ' _p_repr %r' % (e,)
+
         oid = _OGA(self, '_Persistent__oid')
         jar = _OGA(self, '_Persistent__jar')
 
@@ -576,9 +584,9 @@ class Persistent(object):
             except Exception as e:
                 jar_str = ' in %r' % (e,)
 
-        return '<%s.%s object at 0x%x%s%s>' % (
+        return '<%s.%s object at 0x%x%s%s%s>' % (
             type(self).__module__, type(self).__name__, id(self),
-            oid_str, jar_str
+            oid_str, jar_str, p_repr_str
         )
 
 

persistent/tests/test_persistence.py

@@ -1700,6 +1700,7 @@ class _Persistent_Base(object):
     def _normalize_repr(self, r):
         # Pure-python vs C
         r = r.replace('persistent.persistence.Persistent', 'persistent.Persistent')
+        r = r.replace("persistent.tests.test_persistence.", '')
         # addresses
         r = re.sub(r'0x[0-9a-fA-F]*', '0xdeadbeef', r)
         # Python 3.7 removed the trailing , in exception reprs
@@ -1822,9 +1823,61 @@ class _Persistent_Base(object):
         p._p_jar = Jar()
 
         result = self._normalized_repr(p)
-        self.assertEqual(result,
-                         "<persistent.Persistent object at 0xdeadbeef oid b'12345678' in <SomeJar>>")
+        self.assertEqual(
+            result,
+            "<persistent.Persistent object at 0xdeadbeef oid b'12345678' in <SomeJar>>")
+
+    def test__p_repr(self):
+        class P(self._getTargetClass()):
+            def _p_repr(self):
+                return "Override"
+        p = P()
+        self.assertEqual("Override", repr(p))
 
+    def test__p_repr_exception(self):
+        class P(self._getTargetClass()):
+            def _p_repr(self):
+                raise Exception("_p_repr failed")
+        p = P()
+        result = self._normalized_repr(p)
+        self.assertEqual(
+            result,
+            "<P object at 0xdeadbeef"
+            " _p_repr Exception('_p_repr failed')>")
+
+        p._p_oid = b'12345678'
+        result = self._normalized_repr(p)
+        self.assertEqual(
+            result,
+            "<P object at 0xdeadbeef oid b'12345678'"
+            " _p_repr Exception('_p_repr failed')>")
+
+        class Jar(object):
+            def __repr__(self):
+                return '<SomeJar>'
+
+        p._p_jar = Jar()
+        result = self._normalized_repr(p)
+        self.assertEqual(
+            result,
+            "<P object at 0xdeadbeef oid b'12345678'"
+            " in <SomeJar> _p_repr Exception('_p_repr failed')>")
+
+    def test__p_repr_in_instance_ignored(self):
+        class P(self._getTargetClass()):
+            pass
+        p = P()
+        p._p_repr = lambda: "Instance"
+        result = self._normalized_repr(p)
+        self.assertEqual(result, '<P object at 0xdeadbeef>')
+
+    def test__p_repr_baseexception(self):
+        class P(self._getTargetClass()):
+            def _p_repr(self):
+                raise BaseException("_p_repr failed")
+        p = P()
+        with self.assertRaisesRegex(BaseException, '_p_repr failed'):
+            repr(p)
 
 class PyPersistentTests(unittest.TestCase, _Persistent_Base):