Merge pull request #98 from zopefoundation/issue97

Always format 8-byte oids as ints in hexadecimal.

CHANGES.rst

@@ -4,8 +4,15 @@
 4.4.3 (unreleased)
 ------------------
 
-- Nothing changed yet.
-
+- Fix the repr of the persistent objects to include the module name
+  when using the C extension. This matches the pure-Python behaviour
+  and the behaviour prior to 4.4.0. See `issue 92
+  <https://github.com/zopefoundation/persistent/issues/92>`_.
+
+- Change the repr of persistent objects to format the OID as in
+  integer in hexadecimal notation if it is an 8-byte byte string, as
+  ZODB does. This eliminates some issues in doctests. See `issue 95
+  <https://github.com/zopefoundation/persistent/pull/95>`_.
 
 4.4.2 (2018-08-28)
 ------------------

persistent/cPersistence.c

@@ -24,6 +24,16 @@ struct ccobject_head_struct
     CACHE_HEAD
 };
 
+/*
+  The compiler on Windows used for Python 2.7 doesn't include
+  stdint.h.
+*/
+#if !defined(PY3K) && defined(_WIN32)
+typedef unsigned long long uint64_t;
+#else
+#include <stdint.h>
+#endif
+
 /* These two objects are initialized when the module is loaded */
 static PyObject *TimeStamp, *py_simple_new;
 
@@ -1423,10 +1433,16 @@ Per_repr(cPersistentObject *self)
     PyObject *prepr = NULL;
     PyObject *prepr_exc_str = NULL;
 
+    PyObject *module = NULL;
+    PyObject *name = NULL;
     PyObject *oid_str = NULL;
     PyObject *jar_str = NULL;
     PyObject *result = NULL;
 
+    unsigned char* oid_bytes;
+    char buf[20];
+    uint64_t oid_value;
+
     prepr = PyObject_GetAttrString((PyObject*)Py_TYPE(self), "_p_repr");
     if (prepr)
     {
@@ -1446,23 +1462,61 @@ Per_repr(cPersistentObject *self)
         prepr_exc_str = PyUnicode_FromString("");
     }
 
-    oid_str = repr_helper(self->oid, " oid %R");
-    if (!oid_str)
-        goto cleanup;
+    if (self->oid && PyBytes_Check(self->oid) && PyBytes_GET_SIZE(self->oid) == 8) {
+        oid_bytes = (unsigned char*)PyBytes_AS_STRING(self->oid);
+        oid_value = ((uint64_t)oid_bytes[0] << 56)
+            | ((uint64_t)oid_bytes[1] << 48)
+            | ((uint64_t)oid_bytes[2] << 40)
+            | ((uint64_t)oid_bytes[3] << 32)
+            | ((uint64_t)oid_bytes[4] << 24)
+            | ((uint64_t)oid_bytes[5] << 16)
+            | ((uint64_t)oid_bytes[6] << 8)
+            | ((uint64_t)oid_bytes[7]);
+        /*
+          Python's PyUnicode_FromFormat doesn't understand the ll
+          length modifier for %x, so to format a 64-bit value we need to
+          use stdio.
+        */
+        snprintf(buf, sizeof(buf) - 1, "%llx", oid_value);
+        oid_str = PyUnicode_FromFormat(" oid 0x%s", buf);
+    }
+
+    if (!oid_str) {
+        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);
+    module = PyObject_GetAttrString((PyObject*)Py_TYPE(self), "__module__");
+    name = PyObject_GetAttrString((PyObject*)Py_TYPE(self), "__name__");
+
+    if (!module || !name) {
+        /*
+          Some error retrieving __module__ or __name__. Ignore it, use the
+          C data.
+        */
+        PyErr_Clear();
+        result = PyUnicode_FromFormat("<%s object at %p%S%S%S>",
+                                      Py_TYPE(self)->tp_name, self,
+                                      oid_str, jar_str, prepr_exc_str);
+    }
+    else {
+        result = PyUnicode_FromFormat("<%S.%S object at %p%S%S%S>",
+                                      module, 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);
+    Py_XDECREF(name);
+    Py_XDECREF(module);
 
     return result;
 }

persistent/persistence.py

@@ -11,7 +11,7 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-
+import struct
 
 from zope.interface import implementer
 
@@ -52,6 +52,11 @@ SPECIAL_NAMES = ('__class__',
 # check in __getattribute__
 _SPECIAL_NAMES = set(SPECIAL_NAMES)
 
+# Represent 8-byte OIDs as hex integer, just like
+# ZODB does.
+_OID_STRUCT = struct.Struct('>Q')
+_OID_UNPACK = _OID_STRUCT.unpack
+
 @implementer(IPersistent)
 class Persistent(object):
     """ Pure Python implmentation of Persistent base class
@@ -574,7 +579,10 @@ class Persistent(object):
 
         if oid is not None:
             try:
-                oid_str = ' oid %r' % (oid,)
+                if isinstance(oid, bytes) and len(oid) == 8:
+                    oid_str = ' oid 0x%x' % (_OID_UNPACK(oid)[0],)
+                else:
+                    oid_str = ' oid %r' % (oid,)
             except Exception as e:
                 oid_str = ' oid %r' % (e,)
 
@@ -585,7 +593,9 @@ class Persistent(object):
                 jar_str = ' in %r' % (e,)
 
         return '<%s.%s object at 0x%x%s%s%s>' % (
-            type(self).__module__, type(self).__name__, id(self),
+            # Match the C name for this exact class
+            type(self).__module__ if type(self) is not Persistent else 'persistent',
+            type(self).__name__, id(self),
             oid_str, jar_str, p_repr_str
         )
 

persistent/tests/test_persistence.py

@@ -1697,16 +1697,16 @@ class _Persistent_Base(object):
         self.assertEqual(candidate._p_state, GHOST)
         self.assertEqual(candidate.set_by_new, 1)
 
+    # The number 12345678 as a p64, 8-byte string
+    _PACKED_OID = b'\x00\x00\x00\x00\x00\xbcaN'
+    # The number 12345678 printed in hex
+    _HEX_OID = '0xbc614e'
+
     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)
+        r = re.sub(r'at 0x[0-9a-fA-F]*', 'at 0xdeadbeef', r)
         # Python 3.7 removed the trailing , in exception reprs
         r = r.replace("',)", "')")
-        # Python 2 doesn't have a leading b prefix for byte literals
-        r = r.replace("oid '", "oid b'")
         return r
 
     def _normalized_repr(self, o):
@@ -1733,12 +1733,27 @@ class _Persistent_Base(object):
 
     def test_repr_oid_no_jar(self):
         p = self._makeOne()
-        p._p_oid = b'12345678'
+        p._p_oid = self._PACKED_OID
 
         result = self._normalized_repr(p)
         self.assertEqual(
             result,
-            "<persistent.Persistent object at 0xdeadbeef oid b'12345678'>")
+            "<persistent.Persistent object at 0xdeadbeef oid " + self._HEX_OID + ">")
+
+    def test_64bit_oid(self):
+        import struct
+        p = self._makeOne()
+        oid_value = 2 << 62
+        self.assertEqual(oid_value.bit_length(), 64)
+        oid = struct.pack(">Q", oid_value)
+        self.assertEqual(oid, b'\x80\x00\x00\x00\x00\x00\x00\x00')
+
+        p._p_oid = oid
+        result = self._normalized_repr(p)
+        self.assertEqual(
+            result,
+            '<persistent.Persistent object at 0xdeadbeef oid 0x8000000000000000>'
+        )
 
     def test_repr_no_oid_repr_jar_raises_exception(self):
         p = self._makeOne()
@@ -1761,7 +1776,17 @@ class _Persistent_Base(object):
         class BadOID(bytes):
             def __repr__(self):
                 raise Exception("oid repr failed")
-        p._p_oid = BadOID(b'12345678')
+
+        # Our OID is bytes, 8 bytes long. We don't call its repr.
+        p._p_oid = BadOID(self._PACKED_OID)
+
+        result = self._normalized_repr(p)
+        self.assertEqual(
+            result,
+            "<persistent.Persistent object at 0xdeadbeef oid " + self._HEX_OID + ">")
+
+        # Anything other than 8 bytes, though, we do.
+        p._p_oid = BadOID(b'1234567')
 
         result = self._normalized_repr(p)
         self.assertEqual(
@@ -1775,7 +1800,7 @@ class _Persistent_Base(object):
         class BadOID(bytes):
             def __repr__(self):
                 raise Exception("oid repr failed")
-        p._p_oid = BadOID(b'12345678')
+        p._p_oid = BadOID(b'1234567')
 
         class Jar(object):
             def __repr__(self):
@@ -1809,12 +1834,17 @@ class _Persistent_Base(object):
                 raise BaseException("oid repr failed")
         p._p_oid = BadOID(b'12345678')
 
+        # An 8 byte byte string doesn't have repr called.
+        repr(p)
+
+        # Anything other does.
+        p._p_oid = BadOID(b'1234567')
         with self.assertRaisesRegex(BaseException, 'oid repr failed'):
             repr(p)
 
     def test_repr_oid_and_jar(self):
         p = self._makeOne()
-        p._p_oid = b'12345678'
+        p._p_oid = self._PACKED_OID
 
         class Jar(object):
             def __repr__(self):
@@ -1825,7 +1855,7 @@ class _Persistent_Base(object):
         result = self._normalized_repr(p)
         self.assertEqual(
             result,
-            "<persistent.Persistent object at 0xdeadbeef oid b'12345678' in <SomeJar>>")
+            "<persistent.Persistent object at 0xdeadbeef oid " + self._HEX_OID + " in <SomeJar>>")
 
     def test__p_repr(self):
         class P(self._getTargetClass()):
@@ -1842,15 +1872,15 @@ class _Persistent_Base(object):
         result = self._normalized_repr(p)
         self.assertEqual(
             result,
-            "<P object at 0xdeadbeef"
+            "<persistent.tests.test_persistence.P object at 0xdeadbeef"
             " _p_repr Exception('_p_repr failed')>")
 
-        p._p_oid = b'12345678'
+        p._p_oid = self._PACKED_OID
         result = self._normalized_repr(p)
         self.assertEqual(
             result,
-            "<P object at 0xdeadbeef oid b'12345678'"
-            " _p_repr Exception('_p_repr failed')>")
+            "<persistent.tests.test_persistence.P object at 0xdeadbeef oid " + self._HEX_OID
+            + " _p_repr Exception('_p_repr failed')>")
 
         class Jar(object):
             def __repr__(self):
@@ -1860,8 +1890,8 @@ class _Persistent_Base(object):
         result = self._normalized_repr(p)
         self.assertEqual(
             result,
-            "<P object at 0xdeadbeef oid b'12345678'"
-            " in <SomeJar> _p_repr Exception('_p_repr failed')>")
+            "<persistent.tests.test_persistence.P object at 0xdeadbeef oid " + self._HEX_OID
+            + " in <SomeJar> _p_repr Exception('_p_repr failed')>")
 
     def test__p_repr_in_instance_ignored(self):
         class P(self._getTargetClass()):
@@ -1869,7 +1899,8 @@ class _Persistent_Base(object):
         p = P()
         p._p_repr = lambda: "Instance"
         result = self._normalized_repr(p)
-        self.assertEqual(result, '<P object at 0xdeadbeef>')
+        self.assertEqual(result,
+                         '<persistent.tests.test_persistence.P object at 0xdeadbeef>')
 
     def test__p_repr_baseexception(self):
         class P(self._getTargetClass()):