Fix backward compatibility with pickles before v22.2.0 (#1085)

* Remove unnecessary parameterization of tests

* Fix backward compatibility with pickles before v22.2.0

Unfortunately we're using `django-redis` to cache some `attrs` instances
and the change to make `__getstate__` and `__setstate__` use `dict`
values instead of `tuple` values prevents reading any values from the
cache when updating to `attrs>=22.2.0`. We get an `AttributeError`
raised for all attributes.

This happens because the tuple of values stored in the original pickle
is checked for the name of the attribute and so none of the attributes
are set on unpickle.

To address this I've made a change so that `__setstate__` will still be
able to handle values pickled as `tuple`s allowing existing pickles to
be unpickled.

The test just contains a pre-pickled value due to the complexities
around tying to mock methods on a slotted as documented[^1].

I realise that documentation also mentions that we should "think twice"
before using pickle, but sometimes those decisions predate our
involvement in a project! In addition, pretty much all of Django's cache
implementations are built around using pickle by default. This change
will allow an easier path to upgrade by unpickling and repickling all
values in the cache.

Regression in 89a890a326.

[^1]: https://www.attrs.org/en/latest/glossary.html#term-slotted-classes

* Update 1085.change.md

Co-authored-by: Hynek Schlawack <hs@ox.cx>

changelog.d/1085.change.md

@@ -0,0 +1 @@
+Restored ability to unpickle instances pickled before 22.2.0.

src/attr/_make.py

@@ -934,6 +934,12 @@ class _ClassBuilder:
             Automatically created by attrs.
             """
             __bound_setattr = _obj_setattr.__get__(self)
+            if isinstance(state, tuple):
+                # Backward compatibility with attrs instances pickled with
+                # attrs versions before v22.2.0 which stored tuples.
+                for name, value in zip(state_attr_names, state):
+                    __bound_setattr(name, value)
+            else:
                 for name in state_attr_names:
                     if name in state:
                         __bound_setattr(name, state[name])

tests/test_slots.py

@@ -754,13 +754,12 @@ class A:
     c = attr.ib()
 
 
-@pytest.mark.parametrize("cls", [A])
-def test_slots_unpickle_after_attr_removed(cls):
+def test_slots_unpickle_after_attr_removed():
     """
     We don't assign attributes we don't have anymore if the class has
     removed it.
     """
-    a = cls(1, 2, 3)
+    a = A(1, 2, 3)
     a_pickled = pickle.dumps(a)
     a_unpickled = pickle.loads(a_pickled)
     assert a_unpickled == a
@@ -778,12 +777,11 @@ def test_slots_unpickle_after_attr_removed(cls):
         assert not hasattr(new_a, "b")
 
 
-@pytest.mark.parametrize("cls", [A])
-def test_slots_unpickle_after_attr_added(cls, frozen):
+def test_slots_unpickle_after_attr_added(frozen):
     """
     We don't assign attribute we haven't had before if the class has one added.
     """
-    a = cls(1, 2, 3)
+    a = A(1, 2, 3)
     a_pickled = pickle.dumps(a)
     a_unpickled = pickle.loads(a_pickled)
 
@@ -803,3 +801,20 @@ def test_slots_unpickle_after_attr_added(cls, frozen):
         assert new_a.b == 2
         assert new_a.c == 3
         assert not hasattr(new_a, "d")
+
+
+def test_slots_unpickle_is_backward_compatible(frozen):
+    """
+    Ensure object pickled before v22.2.0 can still be unpickled.
+    """
+    a = A(1, 2, 3)
+
+    a_pickled = (
+        b"\x80\x04\x95&\x00\x00\x00\x00\x00\x00\x00\x8c\x10"
+        + a.__module__.encode()
+        + b"\x94\x8c\x01A\x94\x93\x94)\x81\x94K\x01K\x02K\x03\x87\x94b."
+    )
+
+    a_unpickled = pickle.loads(a_pickled)
+
+    assert a_unpickled == a