Add __reduce_ex__ to MutableList; add compat for older pickles

Fixed bug where using ``copy.copy()`` or ``copy.deepcopy()`` on
:class:`.MutableList` would cause the items within the list to be
duplicated, due to an inconsistency in how Python pickle and copy both make
use of ``__getstate__()`` and ``__setstate__()`` regarding lists.  In order
to resolve, a ``__reduce_ex__`` method had to be added to
:class:`.MutableList`.  In order to maintain backwards compatibility with
existing pickles based on ``__getstate__()``, the ``__setstate__()`` method
remains as well; the test suite asserts that pickles made against the old
version of the class can still be deserialized by the pickle module.

Also modified sqlalchemy.testing.util.picklers to return picklers all the way through
pickle.HIGHEST_PROTOCOL.

Fixes: #4603
Change-Id: I7f78b9cfb89d59a706248536c553dc5e1d987b88

diff --git a/doc/build/changelog/unreleased_13/4603.rst b/doc/build/changelog/unreleased_13/4603.rst
new file mode 100644 (file)
index 0000000..a172f36
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4603.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, ext
+    :tickets: 4603
+
+    Fixed bug where using ``copy.copy()`` or ``copy.deepcopy()`` on
+    :class:`.MutableList` would cause the items within the list to be
+    duplicated, due to an inconsistency in how Python pickle and copy both make
+    use of ``__getstate__()`` and ``__setstate__()`` regarding lists.  In order
+    to resolve, a ``__reduce_ex__`` method had to be added to
+    :class:`.MutableList`.  In order to maintain backwards compatibility with
+    existing pickles based on ``__getstate__()``, the ``__setstate__()`` method
+    remains as well; the test suite asserts that pickles made against the old
+    version of the class can still be deserialized by the pickle module.
\ No newline at end of file

diff --git a/lib/sqlalchemy/ext/mutable.py b/lib/sqlalchemy/ext/mutable.py
index eb813a9e8a765777a2d4463d21a0c28c19d739b1..6a7ffd4407ddce5e136a9f8836045e215c7d054b 100644 (file)
--- a/lib/sqlalchemy/ext/mutable.py
+++ b/lib/sqlalchemy/ext/mutable.py
@@ -771,6 +771,14 @@ class MutableList(Mutable, list):
 
     """
 
+    def __reduce_ex__(self, proto):
+        return (self.__class__, (list(self),))
+
+    # needed for backwards compatibility with
+    # older pickles
+    def __setstate__(self, state):
+        self[:] = state
+
     def __setitem__(self, index, value):
         """Detect list set events and emit change events."""
         list.__setitem__(self, index, value)
@@ -838,12 +846,6 @@ class MutableList(Mutable, list):
         else:
             return value
 
-    def __getstate__(self):
-        return list(self)
-
-    def __setstate__(self, state):
-        self[:] = state
-
 
 class MutableSet(Mutable, set):
     """A set type that implements :class:`.Mutable`.

diff --git a/lib/sqlalchemy/testing/util.py b/lib/sqlalchemy/testing/util.py
index 7a2aa6027f8669ccb42efdf26133745ccac65a47..219511ea0e5dd0cef059db2302bf7b1447ebe94c 100644 (file)
--- a/lib/sqlalchemy/testing/util.py
+++ b/lib/sqlalchemy/testing/util.py
@@ -62,7 +62,7 @@ def picklers():
 
     # yes, this thing needs this much testing
     for pickle_ in picklers:
-        for protocol in -1, 0, 1, 2:
+        for protocol in range(-2, pickle.HIGHEST_PROTOCOL):
             yield pickle_.loads, lambda d: pickle_.dumps(d, protocol)
 
 

diff --git a/test/ext/test_mutable.py b/test/ext/test_mutable.py
index eae764d4064a9cacb30581e82c598dcdd40ea214..84b922be7b894f4f44cd2325e5b83418153d1df2 100644 (file)
--- a/test/ext/test_mutable.py
+++ b/test/ext/test_mutable.py
@@ -1,8 +1,12 @@
+import copy
+import pickle
+
 from sqlalchemy import event
 from sqlalchemy import ForeignKey
 from sqlalchemy import func
 from sqlalchemy import Integer
 from sqlalchemy import String
+from sqlalchemy import util
 from sqlalchemy.ext.mutable import MutableComposite
 from sqlalchemy.ext.mutable import MutableDict
 from sqlalchemy.ext.mutable import MutableList
@@ -292,6 +296,16 @@ class _MutableDictTestBase(_MutableDictTestFixture):
 
         eq_(f1.non_mutable_data, {"a": "b"})
 
+    def test_copy(self):
+        f1 = Foo(data={"a": "b"})
+        f1.data = copy.copy(f1.data)
+        eq_(f1.data, {"a": "b"})
+
+    def test_deepcopy(self):
+        f1 = Foo(data={"a": "b"})
+        f1.data = copy.deepcopy(f1.data)
+        eq_(f1.data, {"a": "b"})
+
 
 class _MutableListTestFixture(object):
     @classmethod
@@ -505,6 +519,76 @@ class _MutableListTestBase(_MutableListTestFixture):
         sess.commit()
         eq_(f1.data[0], 3)
 
+    def test_copy(self):
+        f1 = Foo(data=[1, 2])
+        f1.data = copy.copy(f1.data)
+        eq_(f1.data, [1, 2])
+
+    def test_deepcopy(self):
+        f1 = Foo(data=[1, 2])
+        f1.data = copy.deepcopy(f1.data)
+        eq_(f1.data, [1, 2])
+
+    def test_legacy_pickle_loads(self):
+        # due to an inconsistency between pickle and copy, we have to change
+        # MutableList to implement a __reduce_ex__ method.   Which means we
+        # have to make sure all the old pickle formats are still
+        # deserializable since these can be used for persistence. these pickles
+        # were all generated using a MutableList that has only __getstate__ and
+        # __setstate__.
+
+        # f1 = Foo(data=[1, 2])
+        # pickles = [
+        #    dumps(f1.data)
+        #    for loads, dumps in picklers()
+        # ]
+        # print(repr(pickles))
+        # return
+
+        if util.py3k:
+            pickles = [
+                b"\x80\x04\x95<\x00\x00\x00\x00\x00\x00\x00\x8c\x16"
+                b"sqlalchemy.ext.mutable\x94\x8c\x0bMutableList\x94\x93\x94)"
+                b"\x81\x94(K\x01K\x02e]\x94(K\x01K\x02eb.",
+                b"ccopy_reg\n_reconstructor\np0\n(csqlalchemy.ext.mutable\n"
+                b"MutableList\np1\nc__builtin__\nlist\np2\n(lp3\nI1\naI2\n"
+                b"atp4\nRp5\n(lp6\nI1\naI2\nab.",
+                b"ccopy_reg\n_reconstructor\nq\x00(csqlalchemy.ext.mutable\n"
+                b"MutableList\nq\x01c__builtin__\nlist\nq\x02]q\x03(K\x01K"
+                b"\x02etq\x04Rq\x05]q\x06(K\x01K\x02eb.",
+                b"\x80\x02csqlalchemy.ext.mutable\nMutableList\nq\x00)\x81q"
+                b"\x01(K\x01K\x02e]q\x02(K\x01K\x02eb.",
+                b"\x80\x03csqlalchemy.ext.mutable\nMutableList\nq\x00)\x81q"
+                b"\x01(K\x01K\x02e]q\x02(K\x01K\x02eb.",
+                b"\x80\x04\x95<\x00\x00\x00\x00\x00\x00\x00\x8c\x16"
+                b"sqlalchemy.ext.mutable\x94\x8c\x0bMutableList\x94\x93\x94)"
+                b"\x81\x94(K\x01K\x02e]\x94(K\x01K\x02eb.",
+            ]
+        else:
+            pickles = [
+                "\x80\x02csqlalchemy.ext.mutable\nMutableList\nq\x00]q\x01"
+                "(K\x01K\x02e\x85q\x02Rq\x03.",
+                "\x80\x02csqlalchemy.ext.mutable\nMutableList"
+                "\nq\x00]q\x01(K\x01K\x02e\x85q\x02Rq\x03.",
+                "csqlalchemy.ext.mutable\nMutableList\np0\n"
+                "((lp1\nI1\naI2\natp2\nRp3\n.",
+                "csqlalchemy.ext.mutable\nMutableList\nq\x00(]"
+                "q\x01(K\x01K\x02etq\x02Rq\x03.",
+                "\x80\x02csqlalchemy.ext.mutable\nMutableList"
+                "\nq\x01]q\x02(K\x01K\x02e\x85Rq\x03.",
+                "\x80\x02csqlalchemy.ext.mutable\nMutableList\n"
+                "q\x01]q\x02(K\x01K\x02e\x85Rq\x03.",
+                "csqlalchemy.ext.mutable\nMutableList\np1\n"
+                "((lp2\nI1\naI2\natRp3\n.",
+                "csqlalchemy.ext.mutable\nMutableList\nq\x01"
+                "(]q\x02(K\x01K\x02etRq\x03.",
+            ]
+
+        for pickle_ in pickles:
+            obj = pickle.loads(pickle_)
+            eq_(obj, [1, 2])
+            assert isinstance(obj, MutableList)
+
 
 class _MutableSetTestFixture(object):
     @classmethod
@@ -731,6 +815,16 @@ class _MutableSetTestBase(_MutableSetTestFixture):
         sess.commit()
         eq_(f1.data, set([1, 2, 3]))
 
+    def test_copy(self):
+        f1 = Foo(data=set([1, 2]))
+        f1.data = copy.copy(f1.data)
+        eq_(f1.data, set([1, 2]))
+
+    def test_deepcopy(self):
+        f1 = Foo(data=set([1, 2]))
+        f1.data = copy.deepcopy(f1.data)
+        eq_(f1.data, set([1, 2]))
+
 
 class MutableColumnDefaultTest(_MutableDictTestFixture, fixtures.MappedTest):
     @classmethod