From eaa14f6abe1be0c9fc44f41e8630eb8ec3112626 Mon Sep 17 00:00:00 2001 From: Marcin Date: Fri, 26 Apr 2024 17:23:26 +0100 Subject: [PATCH 1/3] add TypeConversionDict.pop method --- CHANGES.rst | 1 + src/werkzeug/datastructures/structures.py | 48 ++++++++++++++++++ src/werkzeug/datastructures/structures.pyi | 34 +++++++++++++ tests/test_datastructures.py | 59 ++++++++++++++++++++-- 4 files changed, 137 insertions(+), 5 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 4c21d209e..c9cd68d60 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,6 +12,7 @@ Unreleased - Add ``stale_while_revalidate`` and ``stale_if_error`` properties to ``ResponseCacheControl``. :issue:`2948` - Add 421 ``MisdirectedRequest`` HTTP exception. :issue:`2850` +- Add ``TypeConversionDict.pop`` method. :issue:`2883` Version 3.0.6 diff --git a/src/werkzeug/datastructures/structures.py b/src/werkzeug/datastructures/structures.py index 4279ceb98..90479091c 100644 --- a/src/werkzeug/datastructures/structures.py +++ b/src/werkzeug/datastructures/structures.py @@ -88,6 +88,54 @@ def get(self, key, default=None, type=None): rv = default return rv + def pop(self, key, default=_missing, type=None): + """Like :meth:`get` but removes the key/value pair. + + >>> d = TypeConversionDict(foo='42', bar='blub') + >>> d.pop('foo', type=int) + 42 + >>> 'foo' in d + False + >>> d.pop('bar', -1, type=int) + -1 + >>> 'bar' in d + False + + :param key: The key to be looked up. + :param default: The default value to be returned if the key is not + in the dictionary. If not further specified it's + an :exc:`KeyError`. + :param type: A callable that is used to cast the value in the dict. + If a :exc:`ValueError` or a :exc:`TypeError` is raised + by this callable the default value is returned. + + .. admonition:: note + + If the type conversion fails, the key is **not** removed from the + dictionary. + + """ + try: + rv = self[key] + except KeyError: + if default is _missing: + raise + return default + if type is not None: + try: + rv = type(rv) + except (ValueError, TypeError): + if default is _missing: + return None + return default + try: + # This method is not meant to be thread-safe, but at least lets not + # fall over if the dict was mutated between the get and the delete. -MK + del self[key] + except KeyError: + pass + return rv + class ImmutableTypeConversionDict(ImmutableDictMixin, TypeConversionDict): """Works like a :class:`TypeConversionDict` but does not support diff --git a/src/werkzeug/datastructures/structures.pyi b/src/werkzeug/datastructures/structures.pyi index 7086ddae1..ef055f4af 100644 --- a/src/werkzeug/datastructures/structures.pyi +++ b/src/werkzeug/datastructures/structures.pyi @@ -36,10 +36,28 @@ class TypeConversionDict(dict[K, V]): def get(self, key: K, default: D, type: Callable[[V], T]) -> D | T: ... @overload def get(self, key: K, type: Callable[[V], T]) -> T | None: ... + @overload + def pop(self, key: K) -> V: ... + @overload + def pop(self, key: K, default: V) -> V: ... + @overload + def pop(self, key: K, default: D) -> V | D: ... + @overload + def pop(self, key: K, default: D, type: Callable[[V], T]) -> D | T: ... + @overload + def pop(self, key: K, type: Callable[[V], T]) -> T | None: ... class ImmutableTypeConversionDict(ImmutableDictMixin[K, V], TypeConversionDict[K, V]): def copy(self) -> TypeConversionDict[K, V]: ... def __copy__(self) -> ImmutableTypeConversionDict[K, V]: ... + @overload + def pop(self, key: K, default: V | None = ...) -> NoReturn: ... + @overload + def pop(self, key: K, default: D) -> NoReturn: ... + @overload + def pop(self, key: K, default: D, type: Callable[[V], T]) -> NoReturn: ... + @overload + def pop(self, key: K, type: Callable[[V], T]) -> NoReturn: ... class MultiDict(TypeConversionDict[K, V]): def __init__( @@ -74,6 +92,14 @@ class MultiDict(TypeConversionDict[K, V]): @overload def pop(self, key: K) -> V: ... @overload + def pop(self, key: K, default: V) -> V: ... + @overload + def pop(self, key: K, default: D) -> V | D: ... + @overload + def pop(self, key: K, default: D, type: Callable[[V], T]) -> D | T: ... + @overload + def pop(self, key: K, type: Callable[[V], T]) -> T | None: ... + @overload def pop(self, key: K, default: V | T = ...) -> V | T: ... def popitem(self) -> tuple[K, V]: ... def poplist(self, key: K) -> list[V]: ... @@ -119,6 +145,14 @@ class OrderedMultiDict(MultiDict[K, V]): @overload def pop(self, key: K) -> V: ... @overload + def pop(self, key: K, default: V) -> V: ... + @overload + def pop(self, key: K, default: D) -> V | D: ... + @overload + def pop(self, key: K, default: D, type: Callable[[V], T]) -> D | T: ... + @overload + def pop(self, key: K, type: Callable[[V], T]) -> T | None: ... + @overload def pop(self, key: K, default: V | T = ...) -> V | T: ... def popitem(self) -> tuple[K, V]: ... def popitemlist(self) -> tuple[K, list[V]]: ... diff --git a/tests/test_datastructures.py b/tests/test_datastructures.py index a681c022b..0ece3a4e7 100644 --- a/tests/test_datastructures.py +++ b/tests/test_datastructures.py @@ -304,6 +304,19 @@ def test_dict_is_hashable(self): assert immutable in x assert immutable2 in x + def test_get_does_not_raise(self): + cls = self.storage_class + immutable = cls({"a": 1}) + assert immutable.get("a") == 1 + + def test_pop_raises(self): + cls = self.storage_class + immutable = cls({"a": 1}) + with pytest.raises(TypeError): + immutable.pop("a") + with pytest.raises(TypeError): + immutable.popitem() + class TestImmutableTypeConversionDict(_ImmutableDictTests): storage_class = ds.ImmutableTypeConversionDict @@ -545,20 +558,56 @@ def test_get_description(self): class TestTypeConversionDict: storage_class = ds.TypeConversionDict - def test_value_conversion(self): + class MyException(Exception): + def raize(self, *args, **kwargs): + raise self + + def test_get_value_conversion(self): d = self.storage_class(foo="1") assert d.get("foo", type=int) == 1 - def test_return_default_when_conversion_is_not_possible(self): + def test_get_return_default_when_conversion_is_not_possible(self): d = self.storage_class(foo="bar", baz=None) assert d.get("foo", default=-1, type=int) == -1 assert d.get("baz", default=-1, type=int) == -1 - def test_propagate_exceptions_in_conversion(self): + def test_get_propagate_exceptions_in_conversion(self): + d = self.storage_class(foo="bar") + with pytest.raises(self.MyException): + d.get("foo", type=lambda x: self.MyException().raize()) + + def test_get_error_in_conversion(self): d = self.storage_class(foo="bar") - switch = {"a": 1} + assert d.get("foo", type=int) is None + + def test_pop_value_conversion(self): + d = self.storage_class(foo="1") + assert d.pop("foo", type=int) == 1 + assert "foo" not in d + + def test_pop_return_when_conversion_is_not_possible(self): + d = self.storage_class(foo="bar", baz=None) + assert d.pop("foo", type=int) is None + assert "foo" in d # key is still in the dict, because the conversion failed + assert d.pop("baz", type=int) is None + assert "baz" in d # key is still in the dict, because the conversion failed + + def test_pop_return_default_when_conversion_is_not_possible(self): + d = self.storage_class(foo="bar", baz=None) + assert d.pop("foo", default=-1, type=int) == -1 + assert "foo" in d # key is still in the dict, because the conversion failed + assert d.pop("baz", default=-1, type=int) == -1 + assert "baz" in d # key is still in the dict, because the conversion failed + + def test_pop_propagate_exceptions_in_conversion(self): + d = self.storage_class(foo="bar") + with pytest.raises(self.MyException): + d.pop("foo", type=lambda x: self.MyException().raize()) + + def test_pop_key_error(self): + d = self.storage_class() with pytest.raises(KeyError): - d.get("foo", type=lambda x: switch[x]) + d.pop("foo") class TestCombinedMultiDict: From 197fcf339f22049d29cf2e4a5ef185331f12b9dd Mon Sep 17 00:00:00 2001 From: David Lord Date: Fri, 25 Oct 2024 15:46:58 -0700 Subject: [PATCH 2/3] pop raises type conversion failure --- src/werkzeug/datastructures/structures.py | 55 ++++++++++++----------- tests/test_datastructures.py | 29 +++++++----- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/src/werkzeug/datastructures/structures.py b/src/werkzeug/datastructures/structures.py index 90479091c..90ed95bc0 100644 --- a/src/werkzeug/datastructures/structures.py +++ b/src/werkzeug/datastructures/structures.py @@ -45,8 +45,8 @@ def __repr__(self): class TypeConversionDict(dict): - """Works like a regular dict but the :meth:`get` method can perform - type conversions. :class:`MultiDict` and :class:`CombinedMultiDict` + """Works like a regular dict but the :meth:`get` and :meth:`pop` methods can + perform type conversions. :class:`MultiDict` and :class:`CombinedMultiDict` are subclasses of this class and provide the same feature. .. versionadded:: 0.5 @@ -89,51 +89,52 @@ def get(self, key, default=None, type=None): return rv def pop(self, key, default=_missing, type=None): - """Like :meth:`get` but removes the key/value pair. + """Similar to :meth:`get` but removes the item, and does not default to + ``None`` if the key doesn't exist. If type conversion fails and no + default is given, the item is not removed and the error is re-raised. - >>> d = TypeConversionDict(foo='42', bar='blub') - >>> d.pop('foo', type=int) + >>> d = TypeConversionDict(a="42", b="abc", c="def") + >>> d.pop("a", type=int) 42 - >>> 'foo' in d + >>> "a" in d False - >>> d.pop('bar', -1, type=int) + >>> d.pop("b", -1, type=int) -1 - >>> 'bar' in d + >>> "b" in d False + >>> d.pop("c", type=int) + ValueError: ... + >>> "c" in d + True :param key: The key to be looked up. - :param default: The default value to be returned if the key is not - in the dictionary. If not further specified it's - an :exc:`KeyError`. - :param type: A callable that is used to cast the value in the dict. - If a :exc:`ValueError` or a :exc:`TypeError` is raised - by this callable the default value is returned. - - .. admonition:: note - - If the type conversion fails, the key is **not** removed from the - dictionary. - + :param default: The value to be returned if the key doesn't exist. If + not given, a ``KeyError`` is raised. + :param type: A callable that is used to convert the value. + If a ``ValueError`` or ``TypeError`` is raised, the default value is + returned if given, otherwise the error is raised. """ + try: + # Don't remove the item yet, type conversion might fail. rv = self[key] except KeyError: if default is _missing: raise + return default + if type is not None: try: rv = type(rv) except (ValueError, TypeError): if default is _missing: - return None + raise + return default - try: - # This method is not meant to be thread-safe, but at least lets not - # fall over if the dict was mutated between the get and the delete. -MK - del self[key] - except KeyError: - pass + + # Remove the item after type conversion succeeds. + del self[key] return rv diff --git a/tests/test_datastructures.py b/tests/test_datastructures.py index 0ece3a4e7..f3fc2ba1b 100644 --- a/tests/test_datastructures.py +++ b/tests/test_datastructures.py @@ -585,19 +585,24 @@ def test_pop_value_conversion(self): assert d.pop("foo", type=int) == 1 assert "foo" not in d - def test_pop_return_when_conversion_is_not_possible(self): - d = self.storage_class(foo="bar", baz=None) - assert d.pop("foo", type=int) is None - assert "foo" in d # key is still in the dict, because the conversion failed - assert d.pop("baz", type=int) is None - assert "baz" in d # key is still in the dict, because the conversion failed + @pytest.mark.parametrize( + ("value", "exc_type"), [("a", ValueError), (None, TypeError)] + ) + def test_pop_conversion_fails(self, value, exc_type): + d = self.storage_class(a=value) - def test_pop_return_default_when_conversion_is_not_possible(self): - d = self.storage_class(foo="bar", baz=None) - assert d.pop("foo", default=-1, type=int) == -1 - assert "foo" in d # key is still in the dict, because the conversion failed - assert d.pop("baz", default=-1, type=int) == -1 - assert "baz" in d # key is still in the dict, because the conversion failed + with pytest.raises(exc_type): + d.pop("a", type=int) + + assert "a" in d + + @pytest.mark.parametrize( + ("value", "exc_type"), [("a", ValueError), (None, TypeError)] + ) + def test_pop_conversion_fails_default(self, value, exc_type): + d = self.storage_class(a=value) + assert d.pop("a", default=None, type=int) is None + assert "a" in d def test_pop_propagate_exceptions_in_conversion(self): d = self.storage_class(foo="bar") From 5dd796553e4a462ee506cc1fbe7f8fda71448a13 Mon Sep 17 00:00:00 2001 From: David Lord Date: Sun, 27 Oct 2024 13:55:06 -0700 Subject: [PATCH 3/3] MultiDict.pop methods take type param --- src/werkzeug/datastructures/structures.py | 92 ++++++++++++++++++----- 1 file changed, 73 insertions(+), 19 deletions(-) diff --git a/src/werkzeug/datastructures/structures.py b/src/werkzeug/datastructures/structures.py index 90ed95bc0..354dccdb5 100644 --- a/src/werkzeug/datastructures/structures.py +++ b/src/werkzeug/datastructures/structures.py @@ -422,9 +422,9 @@ def update(self, mapping): for key, value in iter_multi_items(mapping): MultiDict.add(self, key, value) - def pop(self, key, default=_missing): - """Pop the first item for a list on the dict. Afterwards the - key is removed from the dict, so additional values are discarded: + def pop(self, key, default=_missing, type=None): + """Pop the first item for a list on the dict. If there is more than one + value for the key, the remaining values are discarded. >>> d = MultiDict({"foo": [1, 2, 3]}) >>> d.pop("foo") @@ -432,36 +432,64 @@ def pop(self, key, default=_missing): >>> "foo" in d False - :param key: the key to pop. - :param default: if provided the value to return if the key was - not in the dictionary. + :param key: The key to pop. + :param default: The value to be returned if the key doesn't exist. If + not given, a ``KeyError`` is raised. + :param type: A callable that is used to convert the value. + If a ``ValueError`` or ``TypeError`` is raised, the default value is + returned if given, otherwise the error is raised. """ + try: - lst = dict.pop(self, key) + # Don't remove the item yet, type conversion might fail. + values = dict.__getitem__(self, key) - if len(lst) == 0: - raise exceptions.BadRequestKeyError(key) + if not values: + raise KeyError(key) - return lst[0] + value = values[0] except KeyError: if default is not _missing: return default raise exceptions.BadRequestKeyError(key) from None - def popitem(self): + if type is not None: + try: + value = type(value) + except (ValueError, TypeError): + if default is not _missing: + return default + + raise + + # Remove the item after type conversion succeeds. + del self[key] + return value + + def popitem(self, type=None): """Pop an item from the dict.""" try: - item = dict.popitem(self) + # Let Python pick the item to pop. + key, values = dict.popitem(self) + # Put it back, type conversion might fail. + self.setlist(key, values) - if len(item[1]) == 0: - raise exceptions.BadRequestKeyError(item[0]) + if len(values) == 0: + raise KeyError(key) - return (item[0], item[1][0]) + value = values[0] except KeyError as e: raise exceptions.BadRequestKeyError(e.args[0]) from None - def poplist(self, key): + if type is not None: + value = type(value) + + # Remove the item after type conversion succeeds. + del self[key] + return key, value + + def poplist(self, key, type=None): """Pop the list for a key from the dict. If the key is not in the dict an empty list is returned. @@ -469,15 +497,41 @@ def poplist(self, key): If the key does no longer exist a list is returned instead of raising an error. """ - return dict.pop(self, key, []) + values = dict.pop(self, key, []) - def popitemlist(self): + if type is None: + return values + + out = [] + + for value in values: + try: + out.append(type(value)) + except (ValueError, TypeError): + pass + + return out + + def popitemlist(self, type=None): """Pop a ``(key, list)`` tuple from the dict.""" try: - return dict.popitem(self) + key, values = dict.popitem(self) except KeyError as e: raise exceptions.BadRequestKeyError(e.args[0]) from None + if type is None: + return key, values + + out = [] + + for value in values: + try: + out.append(type(value)) + except (ValueError, TypeError): + pass + + return key, out + def __copy__(self): return self.copy()