don't filter or collapse lists when building urls

Author: davidism

CHANGES.rst

@@ -56,9 +56,11 @@ Unreleased
     convert the value to an int. :issue:`2230`
 -   Always use ``socket.fromfd`` when restarting the dev server.
     :pr:`2287`
--   The behavior of the ``build`` function has been changed,
-    it is no longer flatten a value passed as single-item-list.
-    :issue:`2249`
+-   When passing a dict of URL values to ``Map.build``, list values do
+    not filter out ``None`` or collapse to a single value. Passing a
+    ``MultiDict`` still has these behaviors. This undoes a previous
+    change that made it difficult to pass a list, or ``None`` values in
+    a list, to custom URL converters. :issue:`2249`
 
 
 Version 2.0.3

src/werkzeug/routing.py

@@ -545,10 +545,10 @@ def _prefix_names(src: str) -> ast.stmt:
 _CALL_CONVERTER_CODE_FMT = "self._converters[{elem!r}].to_url()"
 _IF_KWARGS_URL_ENCODE_CODE = """\
 if kwargs:
-    q = '?'
     params = self._encode_query_vars(kwargs)
+    q = "?" if params else ""
 else:
-    q = params = ''
+    q = params = ""
 """
 _IF_KWARGS_URL_ENCODE_AST = _prefix_names(_IF_KWARGS_URL_ENCODE_CODE)
 _URL_ENCODE_AST_NAMES = (_prefix_names("q"), _prefix_names("params"))
@@ -2283,30 +2283,14 @@ def build(
         self.map.update()
 
         if values:
-            temp_values: t.Dict[str, t.Union[t.List[t.Any], t.Any]] = {}
-            always_list = isinstance(values, MultiDict)
-            key: str
-            value: t.Optional[t.Union[t.List[t.Any], t.Any]]
-
-            # For MultiDict, dict.items(values) is like values.lists()
-            # without the call or list coercion overhead.
-            for key, value in dict.items(values):  # type: ignore
-                if value is None:
-                    continue
-
-                if always_list or isinstance(value, (list, tuple)):
-                    value = [v for v in value if v is not None]
-
-                    if not value:
-                        continue
-
-                    if len(value) == 1:
-                        if always_list:
-                            value = value[0]
-
-                temp_values[key] = value
-
-            values = temp_values
+            if isinstance(values, MultiDict):
+                values = {
+                    k: (v[0] if len(v) == 1 else v)
+                    for k, v in dict.items(values)
+                    if len(v) != 0
+                }
+            else:  # plain dict
+                values = {k: v for k, v in values.items() if v is not None}
         else:
             values = {}
 

tests/test_routing.py

@@ -1,4 +1,5 @@
 import gc
+import typing as t
 import uuid
 
 import pytest
@@ -754,48 +755,108 @@ def test_anyconverter():
 
 
 @pytest.mark.parametrize(
-    ("value", "expect"),
+    ("endpoint", "value", "expect"),
     [
-        (None, ""),
-        ([None], ""),
-        ([None, None], ""),
-        ("", "?extra="),
-        ([""], "?extra="),
-        (1.0, "?extra=1.0"),
-        ([1, 2], "?extra=1&extra=2"),
-        ([1, None, 2], "?extra=1&extra=2"),
-        ([1, "", 2], "?extra=1&extra=&extra=2"),
+        ("int", 1, "/1"),
+        ("int", None, r.BuildError),
+        ("int", [1], TypeError),
+        ("list", [1], "/1"),
+        ("list", [1, None, 2], "/1.None.2"),
+        ("list", 1, TypeError),
     ],
 )
-def test_build_append_unknown(value, expect):
-    map = r.Map([r.Rule("/foo/<name>", endpoint="foo")])
-    adapter = map.bind("example.org", "/", subdomain="test")
-    assert (
-        adapter.build("foo", {"name": "bar", "extra": value})
-        == f"http://example.org/foo/bar{expect}"
+def test_build_values_dict(endpoint, value, expect):
+    class ListConverter(r.BaseConverter):
+        def to_url(self, value: t.Any) -> str:
+            return super().to_url(".".join(map(str, value)))
+
+    url_map = r.Map(
+        [r.Rule("/<int:v>", endpoint="int"), r.Rule("/<list:v>", endpoint="list")],
+        converters={"list": ListConverter},
     )
-    assert (
-        adapter.build("foo", {"name": "bar", "extra": value}, append_unknown=False)
-        == "http://example.org/foo/bar"
+    adapter = url_map.bind("localhost")
+
+    if isinstance(expect, str):
+        assert adapter.build(endpoint, {"v": value}) == expect
+    else:
+        with pytest.raises(expect):
+            adapter.build(endpoint, {"v": value})
+
+
+@pytest.mark.parametrize(
+    ("endpoint", "value", "expect"),
+    [
+        ("int", 1, "/1"),
+        ("int", [1], "/1"),
+        ("int", [], r.BuildError),
+        ("int", None, TypeError),
+        ("int", [None], TypeError),
+        ("list", 1, TypeError),
+        ("list", [1], TypeError),
+        ("list", [[1]], "/1"),
+        ("list", [1, None, 2], "/1.None.2"),
+    ],
+)
+def test_build_values_multidict(endpoint, value, expect):
+    class ListConverter(r.BaseConverter):
+        def to_url(self, value: t.Any) -> str:
+            return super().to_url(".".join(map(str, value)))
+
+    url_map = r.Map(
+        [r.Rule("/<int:v>", endpoint="int"), r.Rule("/<list:v>", endpoint="list")],
+        converters={"list": ListConverter},
     )
+    adapter = url_map.bind("localhost")
 
+    if isinstance(expect, str):
+        assert adapter.build(endpoint, MultiDict({"v": value})) == expect
+    else:
+        with pytest.raises(expect):
+            adapter.build(endpoint, MultiDict({"v": value}))
 
-def test_build_append_multiple():
-    map = r.Map([r.Rule("/bar/<float:foo>", endpoint="endp")])
-    adapter = map.bind("example.org", "/", subdomain="subd")
-    params = {"foo": 0.815, "x": [1.0, 3.0], "y": 2.0}
-    a, b = adapter.build("endp", params).split("?")
-    assert a == "http://example.org/bar/0.815"
-    assert set(b.split("&")) == set("y=2.0&x=1.0&x=3.0".split("&"))
+
+@pytest.mark.parametrize(
+    ("value", "expect"),
+    [
+        (None, ""),
+        ([None], ""),
+        ([None, None], ""),
+        ("", "?v="),
+        ([""], "?v="),
+        (0, "?v=0"),
+        (1.0, "?v=1.0"),
+        ([1, 2], "?v=1&v=2"),
+        ([1, None, 2], "?v=1&v=2"),
+        ([1, "", 2], "?v=1&v=&v=2"),
+    ],
+)
+def test_build_append_unknown_dict(value, expect):
+    map = r.Map([r.Rule("/", endpoint="a")])
+    adapter = map.bind("localhost")
+    assert adapter.build("a", {"v": value}) == f"/{expect}"
+    assert adapter.build("a", {"v": value}, append_unknown=False) == "/"
 
 
-def test_build_append_multidict():
-    map = r.Map([r.Rule("/bar/<float:foo>", endpoint="endp")])
-    adapter = map.bind("example.org", "/", subdomain="subd")
-    params = MultiDict((("foo", 0.815), ("x", 1.0), ("x", 3.0), ("y", 2.0)))
-    a, b = adapter.build("endp", params).split("?")
-    assert a == "http://example.org/bar/0.815"
-    assert set(b.split("&")) == set("y=2.0&x=1.0&x=3.0".split("&"))
+@pytest.mark.parametrize(
+    ("value", "expect"),
+    [
+        (None, ""),
+        ([None], ""),
+        ([None, None], ""),
+        ("", "?v="),
+        ([""], "?v="),
+        (0, "?v=0"),
+        (1.0, "?v=1.0"),
+        ([1, 2], "?v=1&v=2"),
+        ([1, None, 2], "?v=1&v=2"),
+        ([1, "", 2], "?v=1&v=&v=2"),
+    ],
+)
+def test_build_append_unknown_multidict(value, expect):
+    map = r.Map([r.Rule("/", endpoint="a")])
+    adapter = map.bind("localhost")
+    assert adapter.build("a", MultiDict({"v": value})) == f"/{expect}"
+    assert adapter.build("a", MultiDict({"v": value}), append_unknown=False) == "/"
 
 
 def test_build_drop_none():