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

4 files changed, 116 insertions, 7 deletions

diff --git a/doc/build/changelog/unreleased_13/4603.rst b/doc/build/changelog/unreleased_13/4603.rst
new file mode 100644
index 000000000..a172f3658
--- /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 eb813a9e8..6a7ffd440 100644
--- 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 7a2aa6027..219511ea0 100644
--- 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 eae764d40..84b922be7 100644
--- 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