starlette icon indicating copy to clipboard operation
starlette copied to clipboard

add support to request.url_for for query parameters

Open cjw296 opened this issue 6 years ago • 14 comments

I totally misunderstood url_for's API, I was expecting url_for('some_route', foo='bar') to return 'http://testserver/my_route/?foo=bar'.

I understand why that is now, but what is the best way to generate urls like this? (for pagination in a rest list api in this case).

Could url_for grow support for query parameters in addition to path parameters?

cjw296 avatar Jun 21 '19 17:06 cjw296

Totally agree with you, it would be very convenient.

I propose to add all unmatched path params to query string.

alex-oleshkevich avatar Jul 10 '19 12:07 alex-oleshkevich

I think what we'll probably do here is have url_for return a URL instance, and add a .with_query_params method to URLs, which would allow for this kind of usage...

url = request.url_for('users', username="tom").with_query_params(...)

lovelydinosaur avatar Nov 06 '19 21:11 lovelydinosaur

Sounds great, but would there be a less wordy alternative?

cjw296 avatar Nov 10 '19 09:11 cjw296

Perhaps just this...

  • request.url_for('users', username="tom").with_query(...)

Or an explicitly shorthand notation...

  • request.url_for('users', username="tom").q(...)

lovelydinosaur avatar Nov 11 '19 12:11 lovelydinosaur

.params() maybe?

cjw296 avatar Nov 11 '19 12:11 cjw296

We probably want to differentiate it more clearly from the properties on the URL class. (.params might just as well indicate a property on the class. .with_query() makes it kinda clear that it's going to return a new URL.)

I think we probably want to end up with...

  • .query_params - A QueryParams datastructure.
  • .query_string - A raw string.
  • .with_query(...) - A new URL.

lovelydinosaur avatar Nov 11 '19 15:11 lovelydinosaur

@cjw296 @tomchristie

I see you're taking some care to think this through, so I'll be closing my open PR on this soon (been open for more than a month now) - the approach I took in the PR was more akin to the existing implementation. But I guess you want to refactor in order to provide a more proper support for the URL's query. And I'm defintely :+1: on that

ricardogsilva avatar Nov 25 '19 10:11 ricardogsilva

are there any updates?

ninyawee avatar Jul 31 '20 09:07 ninyawee

I implemented a naive workaround to this:

def url_for_query(request: Request, name: str, **params: str) -> str:
    url = request.url_for(name)
    parsed = list(urllib.parse.urlparse(url))
    parsed[4] = urllib.parse.urlencode(params)
    return urllib.parse.urlparse(parsed)

templates = Jinja2Templates(directory='templates')
templates.env.globals['url_for_query'] = url_for_query

Then, in template

{ url_for_query('foo', bar='baz') }

This solution doesn't work for routes that accept a parameter.

natanlao avatar Jan 29 '21 07:01 natanlao

I implemented a naive workaround to this:

def url_for_query(request: Request, name: str, **params: str) -> str:
    url = request.url_for(name)
    parsed = list(urllib.parse.urlparse(url))
    parsed[4] = urllib.parse.urlencode(params)
    return urllib.parse.urlparse(parsed)

templates = Jinja2Templates(directory='templates')
templates.env.globals['url_for_query'] = url_for_query

Then, in template

{ url_for_query('foo', bar='baz') }

This solution doesn't work for routes that accept a parameter.

@natanlao Thanks for your code, it's a good solution for me! But I found some typo:

return urllib.parse.urlparse(parsed) -> return urllib.parse.urlunparse(parsed)
{ url_for_query('foo', bar='baz') } -> {{ url_for_query(request, 'foo', bar='baz') }}

setmao avatar Apr 19 '21 11:04 setmao

As per the documentation

Variable arguments that are unknown to the target endpoint are appended to the generated URL as query arguments. So just use as below. url_for('endpointmethod', method_arg="foo", example_query_param="this is query parameter because its not matches with method signature")

gopalsingh462 avatar May 06 '21 09:05 gopalsingh462

@gopalsingh462 Are you referring to the Flask documentation?

natanlao avatar May 06 '21 18:05 natanlao

I think what we'll probably do here is have url_for return a URL instance, and add a .with_query_params method to URLs

I like this idea but I'm concerned that this will change the type signature of url_for in a backwards-incompatible way, as URL is not a subtype of str, the current return value type. It would be easier to place this method on URLPath, but then it would be cumbersome to write url_path_for(...).with_query(...).make_absolute_url(...).

Like @cjw296 I'm coming to this issue for pagination purposes. I want to take the request's URL and replace some offset and limit query parameters while leaving other parameters (e.g., for filtering) intact. Something like this works in the meantime (disclaimer: I haven't tested it much):

from urllib.parse import urlsplit, parse_qs, urlencode

def replace_query_params(url: str, query: dict) -> str:
    _url = urlsplit(url)
    _query = parse_qs(_url.query)
    _query.update(query)
    querystr = urlencode(_query, doseq=True)
    return _url._replace(query=querystr).geturl()

e.g.,

>>> replace_query_params('https://example.com/foo?filter=10&offset=0&limit=50', {'offset': 50})
'https://example.com/foo?filter=10&offset=50&limit=50'

goodmami avatar May 17 '21 08:05 goodmami

Do we have this feature yet?

Sagaryal avatar Nov 23 '21 13:11 Sagaryal

Can we close this @aminalaee ?

Kludex avatar Mar 10 '23 09:03 Kludex

Closing this, since we implemented https://github.com/encode/starlette/pull/1385. Let me know if that's not enough, and we can reopen.

Kludex avatar Jun 20 '23 19:06 Kludex

It is enough. Thank you

aminalaee avatar Jun 20 '23 19:06 aminalaee