Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

routing: Transparently convert a MultiDict containing parameters. #724

Closed
wants to merge 3 commits into from
Closed
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -29,6 +29,8 @@ Version 0.10.5

- Reloader: Correctly detect file changes made by moving temporary files over
the original, which is e.g. the case with PyCharm (pull request ``#722``).
- Reintroduce `MultiDict` values for building URLs. We actually can eat our
cake and have it too.

Version 0.10.4
--------------
@@ -100,8 +100,6 @@ def test_basic_building():
assert adapter.build('bar', {'baz': 'blub'}) == \
'http://example.org/bar/blub'
assert adapter.build('bari', {'bazi': 50}) == 'http://example.org/bar/50'
multivalues = MultiDict([('bazi', 50), ('bazi', None)])
assert adapter.build('bari', multivalues) == 'http://example.org/bar/50'
assert adapter.build('barf', {'bazf': 0.815}) == \
'http://example.org/bar/0.815'
assert adapter.build('barp', {'bazp': 'la/di'}) == \
@@ -482,13 +480,25 @@ def test_build_append_unknown():

def test_build_append_multiple():
map = r.Map([
r.Rule('/bar/<float:bazf>', endpoint='barf')
r.Rule('/bar/<float:foo>', endpoint='endp')
])
adapter = map.bind('example.org', '/', subdomain='blah')
params = {'bazf': 0.815, 'bif': [1.0, 3.0], 'pof': 2.0}
a, b = adapter.build('barf', params).split('?')
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('&'))


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('pof=2.0&bif=1.0&bif=3.0'.split('&'))
assert set(b.split('&')) == set('y=2.0&x=1.0&x=3.0'.split('&'))


def test_method_fallback():
@@ -1717,6 +1717,12 @@ def build(self, endpoint, values=None, method=None, force_external=False,
>>> urls.build("index", {'q': ['a', 'b', 'c']})
'/?q=a&q=b&q=c'
If an actual :py:class:`werkzeug.datastructures.MultiDict` is passed
in it is automatically expanded to do the correct thing:
>>> urls.build("index", MultiDict((('p', 'z'), ('q', 'a'), ('q', 'b'))))
'/?p=z&q=a&q=b'
If a rule does not exist when building a `BuildError` exception is
raised.
@@ -1742,10 +1748,14 @@ def build(self, endpoint, values=None, method=None, force_external=False,
self.map.update()
if values:
if isinstance(values, MultiDict):
valueiter = iteritems(values, multi=True)
temp = dict(
(k, (values.getlist(k)
if len(values.getlist(k)) > 1

This comment has been minimized.

Copy link
@untitaker

untitaker Apr 10, 2015

Member

This is really slow, it calls getlist twice for no reason. I'll try to write some benchmarks, but the current code isn't acceptable (though I apprechiate it that there are no perf regressions for the non-multidict path)

This comment has been minimized.

Copy link
@houseofsuns

houseofsuns Apr 10, 2015

Author Contributor

What about the following? It is imho a tad uglier, but should be pretty optimal in performance (in that there is nothing done which could be spared and keep the functionality the same).

temp = {}
for key, list_value in values.lists():
    if len(list_value) == 1:
        temp[key] = list_value[0]
    else:
        temp[key] = list_value

This comment has been minimized.

Copy link
@untitaker

untitaker Apr 11, 2015

Member

This looks good but please use iterlists from compat.

else values[k]))
for k in values)
else:
valueiter = iteritems(values)
values = dict((k, v) for k, v in valueiter if v is not None)
temp = values
values = dict((k, v) for k, v in iteritems(temp) if v is not None)
else:
values = {}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.