Merge pull request #84 from zopefoundation/issue11

Include OID and jar in default repr of Persistent; let subclasses implement _p_repr for exception safety

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

@@ -1372,6 +1372,101 @@ Per_set_sticky(cPersistentObject *self, PyObject* value)
     return 0;
 }
 
+static PyObject*
+repr_format_exception(char* format)
+{
+    /* 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)
+            result = repr_format_exception(format);
+    }
+    else
+    {
+        result = PyUnicode_FromString("");
+    }
+
+    return result;
+
+}
+
+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)
+        goto cleanup;
+
+    jar_str = repr_helper(self->jar, " in %R");
+    if (!jar_str)
+        goto cleanup;
+
+    result = PyUnicode_FromFormat("<%s object at %p%S%S%S>",
+                                  Py_TYPE(self)->tp_name, self,
+                                  oid_str, jar_str, prepr_exc_str);
+
+cleanup:
+    Py_XDECREF(prepr);
+    Py_XDECREF(prepr_exc_str);
+    Py_XDECREF(oid_str);
+    Py_XDECREF(jar_str);
+
+    return result;
+}
+
 static PyGetSetDef Per_getsets[] = {
   {"_p_changed", (getter)Per_get_changed, (setter)Per_set_changed},
   {"_p_jar", (getter)Per_get_jar, (setter)Per_set_jar},
@@ -1454,7 +1549,7 @@ static PyTypeObject Pertype = {
   0,                                /* tp_getattr */
   0,                                /* tp_setattr */
   0,                                /* tp_compare */
-  0,                                /* tp_repr */
+  (reprfunc)Per_repr,               /* tp_repr */
   0,                                /* tp_as_number */
   0,                                /* tp_as_sequence */
   0,                                /* tp_as_mapping */

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

@@ -11,7 +11,7 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-import sys
+
 
 from zope.interface import implementer
 
@@ -20,14 +20,13 @@ from persistent.interfaces import GHOST
 from persistent.interfaces import UPTODATE
 from persistent.interfaces import CHANGED
 from persistent.interfaces import STICKY
-from persistent.interfaces import OID_TYPE
+
 from persistent.interfaces import SERIAL_TYPE
 from persistent.timestamp import TimeStamp
 from persistent.timestamp import _ZERO
 from persistent._compat import copy_reg
 from persistent._compat import intern
 
-from . import ring
 
 _INITIAL_SERIAL = _ZERO
 
@@ -558,6 +557,39 @@ class Persistent(object):
         if cache is not None:
             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')
+
+        oid_str = ''
+        jar_str = ''
+
+        if oid is not None:
+            try:
+                oid_str = ' oid %r' % (oid,)
+            except Exception as e:
+                oid_str = ' oid %r' % (e,)
+
+        if jar is not None:
+            try:
+                jar_str = ' in %r' % (jar,)
+            except Exception as e:
+                jar_str = ' in %r' % (e,)
+
+        return '<%s.%s object at 0x%x%s%s%s>' % (
+            type(self).__module__, type(self).__name__, id(self),
+            oid_str, jar_str, p_repr_str
+        )
+
+
 def _estimated_size_in_24_bits(value):
     if value > 1073741696:
         return 16777215