100% coverage for attrhooks.py

Two bug fixes for pure-Python mode: deleting _p_oid and deleting *any* _p attribute.

CHANGES.rst

@@ -6,6 +6,11 @@
 
 - Add support for Python 3.7 and drop support for Python 3.3.
 
+- Fix deleting the ``_p_oid`` of a pure-Python persistent object when
+  it is in a cache.
+
+- Fix deleting special (``_p``) attributes of a pure-Python persistent
+  object that overrides ``__delattr__`` and correctly calls ``_p_delattr``.
 
 4.3.0 (2018-07-30)
 ------------------

docs/api/attributes.rst

@@ -309,3 +309,21 @@ object as changed if the name starts with ``tmp_``:
    Traceback (most recent call last):
    ...
    AttributeError: tmp_z
+
+If we attempt to delete ``_p_oid``, we find that we can't, and the
+object is also not activated or changed:
+
+.. doctest::
+
+   >>> del o._p_oid
+   Traceback (most recent call last):
+   ...
+   ValueError: can't delete _p_oid of cached object
+   >>> o._p_changed
+   False
+
+We are allowed to delete ``_p_changed``, which sets it to ``None``:
+
+   >>> del o._p_changed
+   >>> o._p_changed is None
+   True

persistent/persistence.py

@@ -38,6 +38,7 @@
 
 _OGA = object.__getattribute__
 _OSA = object.__setattr__
+_ODA = object.__delattr__
 
 # These names can be used from a ghost without causing it to be
 # activated. These are standardized with the C implementation
@@ -301,7 +302,7 @@ def __delattr__(self, name):
                 if (_OGA(self, '_Persistent__jar') is not None and
                     _OGA(self, '_Persistent__oid') is not None):
                     _OGA(self, '_p_register')()
-        object.__delattr__(self, name)
+        _ODA(self, name)
 
     def _slotnames(self, _v_exclude=True):
         slotnames = copy_reg._slotnames(type(self))
@@ -477,7 +478,12 @@ def _p_delattr(self, name):
         """ See IPersistent.
         """
         if name.startswith('_p_'):
-            delattr(self, name)
+            if name == '_p_oid' and self._p_is_in_cache(_OGA(self, '_Persistent__jar')):
+                # The C implementation forbids deleting the oid
+                # if we're already in a cache. Match its error message
+                raise ValueError('can not change _p_jar of cached object')
+
+            _ODA(self, name)
             return True
         self._p_activate()
         self._p_accessed()

persistent/tests/attrhooks.py

@@ -35,9 +35,8 @@ def __getattr__(self, name):
         """
         # Don't pretend we have any special attributes.
         if name.startswith("__") and name.endswrith("__"):
-            raise AttributeError(name)
-        else:
-            return name.upper(), self._p_changed
+            raise AttributeError(name) # pragma: no cover
+        return name.upper(), self._p_changed
 
 
 class VeryPrivate(Persistent):

persistent/tests/cucumbers.py

@@ -14,8 +14,6 @@
 # Example objects for pickling.
 
 from persistent import Persistent
-from persistent._compat import PYTHON2
-
 
 def print_dict(d):
     d = sorted(d.items())
@@ -24,18 +22,14 @@ def print_dict(d):
         )))
 
 def cmpattrs(self, other, *attrs):
+    result = 0
     for attr in attrs:
         if attr[:3] in ('_v_', '_p_'):
-            continue
-        lhs, rhs = getattr(self, attr, None), getattr(other, attr, None)
-        if PYTHON2:
-           c = cmp(lhs, rhs)
-           if c:
-               return c
-        else:
-            if lhs != rhs:
-                return 1
-    return 0
+            raise AssertionError("_v_ and _p_ attrs not allowed")
+        lhs = getattr(self, attr, None)
+        rhs = getattr(other, attr, None)
+        result += lhs != rhs
+    return result
 
 class Simple(Persistent):
     def __init__(self, name, **kw):
@@ -83,7 +77,7 @@ def __init__(self, s1, s2):
 
     @property
     def _attrs(self):
-        return list(self.__dict__.keys())
+        raise NotImplementedError()
 
     def __eq__(self, other):
         return cmpattrs(self, other, '__class__', *self._attrs) == 0