starlette
starlette copied to clipboard
encode
Let Request.url_for lookup routes in mounted app first
Proposed fix for https://github.com/encode/starlette/issues/814
This modifies the lookup of routes by name to first look in the routes of the mounted app before resolving routes in the root app.
Motivation
As mentioned by @jussiarpalahti in by https://github.com/encode/starlette/issues/814#issuecomment-593104008, this lookup algorithm makes more sense if the mounted sub-application is maintained as an independent module.
In that case you would not expect the sub-application to know under what name (if any) it is mounted.
With this feature, you can even mount the same application twice under different routes, without any need to configure the mount prefix as an application property.
@plankthom Thank you for the PR. Can you please explain a bit about this?
As far as I could see this was not really a bug, this was the intended behaviour.
For doing reverse URL lookup on sub-mounted apps you'd need request.url_for('sub:<name>') and calling request.url_for(<name>) should not work.
@aminalaee : I`ve added some motivation in the PR description.
A FastApi example
test/trinkets.py
from fastapi import FastAPI, Request
app = FastAPI(title='Trinkets')
trinkets = [ {'name': 'nice'}, {'name': 'colorful'} ]
@app.get('/')
def info(request: Request):
return {
'_links: {'trinkets': {'href': request.url_for('list_trinkets')}}
}
@app.get('/trinket')
def list_trinkets(request: Request):
return [
{
**trinket,
'_links': {'self': {'href': request.url_for('get_trinket', trinket_name=trinket['name'])}}
}
for trinket in trinkets
]
@app.get('/trinket/{trinket_name}')
def get_trinket(trinket_name: str, request: Request):
return next(t for t in trinkets if t['name'] == trinket_name)
test/main.py
import fastapi
import trinket
app = fastapi.FastAPI(title='Trinkets and Treasures')
@app.get('/')
def info(request: fastapi.Request):
return {
'trinkets_v1' : request.url_for('v1:list_trinkets'),
'trinkets_v2' : request.url_for('v2:list_trinkets')
}
app.mount('/api/v1', app=trinket.app, name= 'v1')
app.mount('/api/v2', app=trinket.app, name= 'v2')
curl http://localhost:8000 | jq
{
"trinkets_v1": "http://localhost:8000/api/v1/trinket",
"trinkets_v2": "http://localhost:8000/api/v2/trinket"
}
curl curl http://localhost:8000/api/v2/trinket
[
{
"name": "nice",
"_links": {
"self": {
"href": "http://localhost:8000/api/v2/trinket/nice"
}
}
},
{
"name": "colorful",
"_links": {
"self": {
"href": "http://localhost:8000/api/v2/trinket/colorful"
}
}
}
]
Note that current fastapi treats these sub-apps independently in their documentation (e.g. documented with the local paths):
http://localhost:8000/api/v1/docs
(the current fastapi docs on that topic lag a bit behind)
Hi. Original issue author here. Thank you @plankthom for looking into this.
My original use case was to make several small, reusable ASGI apps and combine them into larger app instances. Their URLs and URL namespace could differ depending on needs of the main app. There shouldn't be a need for any of the sub-apps to know where they reside, if ASGI framework can manage their URLs. Examples of such apps I had was authorization, and pub/sub messaging.
@aminalaee did previous comments clarify the intent of this PR? If so would you consider reviewing it?
@Kludex I want this too. This is smth I suffer too.
Here is my example: https://github.com/alex-oleshkevich/ohmyadmin/blob/master/ohmyadmin/resources.py#L485
App can have many Resource classes mounted under different paths. And it is impossible to generate a valid URL path to the route within the mounted resource by name. I had to wrap it in a hackish name=self.url_name('edit') to add a unique prefix.
See
class Resource(Router):
def __init__():
super().__init__(routes=[
Route('/', self.index_view, name='index'),
Route('/edit', self.edit_view, name='edit'),
])
app = Starlette(routes=[
Mount('/app/products', Resource()),
Mount('/app/users', Resource()),
Mount('...', Resource()),
])
There is no easy way to generate a URL path for edit from index and vice versa:
def index_view(self, request):
return self.request.url_path(???) # 'edit' is a global name
You have to prefix route names to make them unique:
Route('/', self.index_view, name='products_index'),
Route('/', self.index_view, name='users_index'),
And do this for all. This is error-prone and too verbose.
If you decide to namespace URL in Mount everything worsens. You cannot generate URL to /app/users without knowing the namespace name defined by the user (is it 'users' in this example).
Mount('/app/users', Resource(), name='users'),
# in view code of 3rd-party library
def index_view(self, request):
return self.request.url_path(namespace???:routename???) # 'edit' is a global name
I think we can do something like this:
self.request.url_path('.edit')
self.request.url_path('.index')
If route name starts with dot, then we look up the current ASGI app first (inner router for example) and then iterate parents"
self.request.url_path('.edit') # works, returns `/api/products/edit`
self.request.url_path('.index') # works, returns `/api/products`
self.request.url_path('home') # works, returns `/`
self.request.url_path('index') # works, returns url for `index` route name, note it is not prefixed with dot.
The Flask does it in the same way with blueprints - https://flask.palletsprojects.com/en/2.2.x/blueprints/#building-urls
@Kludex @alex-oleshkevich I can update this PR with the proposal (use . prefixed paths to have resolution local to the inner router) ... This will indeed improve backward compatibility, although IMO there is very little risk at incompatibility in the current proposal (other than that previously broken lookups would work now).
Closing it as stale since no one replied my last question.
Happy to reopen. 👍