Fix slicing and copying of PersistentList and PersistentMapping.

See https://bugs.python.org/issue27639 and https://bugs.python.org/issue27141

Fixes #112

CHANGES.rst

@@ -4,6 +4,13 @@
 4.5.2 (unreleased)
 ------------------
 
+- Fix slicing of ``PersistentList`` to always return instances of the
+  same class. It was broken on Python 3 prior to 3.7.4.
+
+- Fix copying  of ``PersistentList`` and ``PersistentMapping`` using
+  ``copy.copy`` to also copy the underlying data object. This was
+  broken prior to Python 3.7.4.
+
 - Fixed ``PersistentList`` to mark itself as changed after calling
   ``clear``. See `PR 115 <https://github.com/zopefoundation/persistent/pull/115/>`_.
 

persistent/list.py

@@ -34,7 +34,10 @@ class PersistentList(UserList, persistent.Persistent):
        Using the `clear` method, or writing ``del inst[:]`` now only
        results in marking the instance as changed if it actually removed
        items.
+    .. versionchanged:: 4.5.2
+       The `copy` method is available on Python 2.
     """
+    __super_getitem = UserList.__getitem__
     __super_setitem = UserList.__setitem__
     __super_delitem = UserList.__delitem__
     __super_iadd = UserList.__iadd__
@@ -52,6 +55,29 @@ class PersistentList(UserList, persistent.Persistent):
         else lambda inst: inst.__delitem__(slice(None, None, None))
     )
 
+    if (2, 7, 0) < sys.version_info[:3] < (3, 7, 4):
+        # Prior to 3.7.4, Python 3 failed to properly
+        # return an instance of the same class.
+        # See https://bugs.python.org/issue27639
+        # and https://github.com/zopefoundation/persistent/issues/112.
+        # We only define the special method on the necessary versions to avoid
+        # any speed penalty.
+        def __getitem__(self, item):
+            result = self.__super_getitem(item)
+            if isinstance(item, slice):
+                result = self.__class__(result)
+            return result
+
+    if sys.version_info[:3] < (3, 7, 4):
+        # Likewise for __copy__.
+        # See https://github.com/python/cpython/commit/3645d29a1dc2102fdb0f5f0c0129ff2295bcd768
+        def __copy__(self):
+            inst = self.__class__.__new__(self.__class__)
+            inst.__dict__.update(self.__dict__)
+            # Create a copy and avoid triggering descriptors
+            inst.__dict__["data"] = self.__dict__["data"][:]
+            return inst
+
     def __setitem__(self, i, item):
         self.__super_setitem(i, item)
         self._p_changed = 1
@@ -68,6 +94,9 @@ def __delitem__(self, i):
         __super_setslice = UserList.__setslice__
         __super_delslice = UserList.__delslice__
 
+        def copy(self):
+            return self.__class__(self)
+
         def __setslice__(self, i, j, other):
             self.__super_setslice(i, j, other)
             self._p_changed = 1

persistent/mapping.py

@@ -16,6 +16,8 @@
 
 $Id$"""
 
+import sys
+
 import persistent
 from persistent._compat import IterableUserDict
 
@@ -55,6 +57,17 @@ class PersistentMapping(IterableUserDict, persistent.Persistent):
     __super_pop = IterableUserDict.pop
     __super_popitem = IterableUserDict.popitem
 
+    if sys.version_info[:3] < (3, 7, 4):
+        # See PersistentList.
+        # See https://github.com/python/cpython/commit/3645d29a1dc2102fdb0f5f0c0129ff2295bcd768
+        def __copy__(self):
+            inst = self.__class__.__new__(self.__class__)
+            inst.__dict__.update(self.__dict__)
+            # Create a copy and avoid triggering descriptors
+            data = self.__dict__['data'] if 'data' in self.__dict__ else self.__dict__['_containers']
+            inst.__dict__["data"] = data.copy()
+            return inst
+
     def __delitem__(self, key):
         self.__super_delitem(key)
         self._p_changed = 1

persistent/tests/test_list.py

@@ -16,6 +16,8 @@
 
 import unittest
 
+from persistent.tests.utils import copy_test
+
 l0 = []
 l1 = [0]
 l2 = [0, 1]
@@ -347,6 +349,32 @@ def test_reverse(self):
         self.assertEqual(inst, [1, 2])
         self.assertTrue(inst._p_changed)
 
+    def test_getslice_same_class(self):
+        class MyList(self._getTargetClass()):
+            pass
+
+        inst = MyList()
+        inst._p_jar = self._makeJar()
+        # Entire thing, empty.
+        inst2 = inst[:]
+        self.assertIsNot(inst, inst2)
+        self.assertEqual(inst, inst2)
+        self.assertIsInstance(inst2, MyList)
+        # The _p_jar is *not* propagated.
+        self.assertIsNotNone(inst._p_jar)
+        self.assertIsNone(inst2._p_jar)
+
+        # Partial
+        inst.extend((1, 2, 3))
+        inst2 = inst[1:2]
+        self.assertEqual(inst2, [2])
+        self.assertIsInstance(inst2, MyList)
+        self.assertIsNone(inst2._p_jar)
+
+    def test_copy(self):
+        inst = self._makeOne()
+        inst.append(42)
+        copy_test(self, inst)
 
 def test_suite():
     return unittest.defaultTestLoader.loadTestsFromName(__name__)

persistent/tests/test_mapping.py

@@ -13,6 +13,8 @@
 ##############################################################################
 import unittest
 
+from persistent.tests.utils import copy_test
+
 # pylint:disable=blacklisted-name, protected-access
 
 class Test_default(unittest.TestCase):
@@ -221,6 +223,10 @@ def test___repr___converts_legacy_container_attr(self):
         self.assertEqual(pm.__dict__, {'data': {'a': 1}})
         self.assertEqual(pm.__getstate__(), {'data': {'a': 1}})
 
+    def test_copy(self):
+        pm = self._makeOne()
+        pm['key'] = 42
+        copy_test(self, pm)
 
 class Test_legacy_PersistentDict(unittest.TestCase):
 

persistent/tests/utils.py

@@ -62,3 +62,18 @@ def setstate(self, obj):
         # This isn't what setstate() is supposed to do,
         # but it suffices for the tests.
         obj.__setstate__(self.remembered)
+
+
+def copy_test(self, obj):
+    import copy
+    # Test internal copy
+    obj_copy = obj.copy()
+    self.assertIsNot(obj.data, obj_copy.data)
+    self.assertEqual(obj.data, obj_copy.data)
+
+    # Test copy.copy
+    obj.test = [1234]  # Make sure instance vars are also copied.
+    obj_copy = copy.copy(obj)
+    self.assertIsNot(obj.data, obj_copy.data)
+    self.assertEqual(obj.data, obj_copy.data)
+    self.assertIs(obj.test, obj_copy.test)