From 966b5b49b6521483f1c90b4499c4c80e80136de3 Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Mon, 5 Oct 2020 15:57:47 +0100 Subject: [PATCH] Updated MultiValueDict.update() to mirror dict.update() behavior. Changes in behavior include: - Accepting iteration over empty sequences, updating nothing. - Accepting iterable of 2-tuples providing key-value pairs. - Failing with the same or comparable exceptions for invalid input. Notably this replaces the previous attempt to catch TypeError which was unreachable as the call to .items() resulted in AttributeError on non-dict objects. --- django/utils/datastructures.py | 15 ++++++------ tests/utils_tests/test_datastructures.py | 29 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/django/utils/datastructures.py b/django/utils/datastructures.py index 740ad5a86d..871b016715 100644 --- a/django/utils/datastructures.py +++ b/django/utils/datastructures.py @@ -194,16 +194,15 @@ class MultiValueDict(dict): if len(args) > 1: raise TypeError("update expected at most 1 argument, got %d" % len(args)) if args: - other_dict = args[0] - if isinstance(other_dict, MultiValueDict): - for key, value_list in other_dict.lists(): + arg = args[0] + if isinstance(arg, MultiValueDict): + for key, value_list in arg.lists(): self.setlistdefault(key).extend(value_list) else: - try: - for key, value in other_dict.items(): - self.setlistdefault(key).append(value) - except TypeError: - raise ValueError("MultiValueDict.update() takes either a MultiValueDict or dictionary") + if isinstance(arg, Mapping): + arg = arg.items() + for key, value in arg: + self.setlistdefault(key).append(value) for key, value in kwargs.items(): self.setlistdefault(key).append(value) diff --git a/tests/utils_tests/test_datastructures.py b/tests/utils_tests/test_datastructures.py index 940c8c9e7d..45d172f984 100644 --- a/tests/utils_tests/test_datastructures.py +++ b/tests/utils_tests/test_datastructures.py @@ -195,6 +195,35 @@ class MultiValueDictTests(SimpleTestCase): x.update(a=4, b=5) self.assertEqual(list(x.lists()), [('a', [1, 4]), ('b', [2, 5]), ('c', [3])]) + def test_update_with_empty_iterable(self): + for value in ['', b'', (), [], set(), {}]: + d = MultiValueDict() + d.update(value) + self.assertEqual(d, MultiValueDict()) + + def test_update_with_iterable_of_pairs(self): + for value in [(('a', 1),), [('a', 1)], {('a', 1)}]: + d = MultiValueDict() + d.update(value) + self.assertEqual(d, MultiValueDict({'a': [1]})) + + def test_update_raises_correct_exceptions(self): + # MultiValueDict.update() raises equivalent exceptions to + # dict.update(). + # Non-iterable values raise TypeError. + for value in [None, True, False, 123, 123.45]: + with self.subTest(value), self.assertRaises(TypeError): + MultiValueDict().update(value) + # Iterables of objects that cannot be unpacked raise TypeError. + for value in [b'123', b'abc', (1, 2, 3), [1, 2, 3], {1, 2, 3}]: + with self.subTest(value), self.assertRaises(TypeError): + MultiValueDict().update(value) + # Iterables of unpackable objects with incorrect number of items raise + # ValueError. + for value in ['123', 'abc', ('a', 'b', 'c'), ['a', 'b', 'c'], {'a', 'b', 'c'}]: + with self.subTest(value), self.assertRaises(ValueError): + MultiValueDict().update(value) + class ImmutableListTests(SimpleTestCase):