From: Mike Bayer Date: Tue, 9 Apr 2019 21:38:53 +0000 (-0400) Subject: Add __reduce_ex__ to MutableList; add compat for older pickles X-Git-Tag: rel_1_3_3~5^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=21099479daf98dca84cb97e928951ea0c486b479;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..a172f36587 --- /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 eb813a9e8a..6a7ffd4407 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 7a2aa6027f..219511ea0e 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 eae764d406..84b922be7b 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