Opened 15 years ago
Closed 13 years ago
#11559 closed New feature (fixed)
urlresolvers.reverse do not work with namespaced urls and captured parameters in parent urlconf
Reported by: | Owned by: | Harro | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | dceu2011 |
Cc: | Rolando Espinoza La fuente, kmike84@…, Alexander Koshelev, me@…, Florian Apolloner, trey@…, t.kaemming+t11559@…, gnublade@…, remco@…, trowbrds@…, chipx86@…, ungenio@…, Michael van Tellingen, jeffrey@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Urlresolvers.reverse (and {% url .. %} template tag) do not work with namespaced urls and captured parameters in parent urlconf.
This works:
# In urls.py urlpatterns = patterns('', (r'^(?P<username>\w+)/blog/', include('foo.urls')), ) # In foo/urls.py urlpatterns = patterns('foo.views', url(r'^index$', 'blog.index', name='blog_index'), url(r'^archive/$', 'blog.archive', name='blog_archive'), ) #in some view from django.core.urlresolvers import reverse url = reverse('blog_index', args=['john'])
And this doesn't:
# In urls.py urlpatterns = patterns('', (r'^(?P<username>\w+)/blog/', include('foo.urls', namespace='user_blogs', app_name='foo')), ) # In foo/urls.py urlpatterns = patterns('foo.views', url(r'^index$', 'blog.index', name='blog_index'), url(r'^archive/$', 'blog.archive', name='blog_archive'), ) #in some view from django.core.urlresolvers import reverse url = reverse('user_blogs:blog_index', args=['john'], current_app='foo')
This works again:
# In urls.py urlpatterns = patterns('', (r'^(?P<username>\w+)/blog/', include('foo.urls', namespace='user_blogs', app_name='foo')), ) # In foo/urls.py urlpatterns = patterns('foo.views', url(r'^index$', 'blog.index', name='blog_index'), url(r'^archive/$', 'blog.archive', name='blog_archive'), ) #in some view from django.core.urlresolvers import reverse url = reverse('user_blogs:blog_index', current_app='foo') # but `url` variable contains "/(?P<username>\\w+)/blog/index"
Attachments (4)
Change History (36)
comment:1 by , 15 years ago
milestone: | → 1.1 |
---|
comment:2 by , 15 years ago
milestone: | 1.1 |
---|---|
Triage Stage: | Unreviewed → Someday/Maybe |
It's arguable whether this is a bug or a feature request.
The URL namespace feature was designed to remove ambiguity in the deployment of multiple instances of an application. To that end, the namespace feature works exactly as intended.
What is being proposed here is a desire to parameterize the deployable application itself. In the example provided, the 'blog' application provides an index and archive view, but is being parameterized using the username. By contrast, the admin app is entirely self contained - it has no external parameterization, only a static 'mount point' for the URL space.
The provided URL sample doesn't require namespacing at all, since there is only one instance of the app. You would only require namespacing if there were two instances of the blog application:
/foo/<username>/blog/index /archive /bar/<username>/blog/index /archive
While I can see how this could be an interesting addition to namespaced URLs, it isn't really the same problem faced by the admin app. It also opens up a bunch of questions regarding the manner in which the parameters are passed to an application - in this case, the blog requires that all views in the app take a username argument, and conversely, that it must be deployed in a tree that provides a captured username parameter. Given the potential for error here, Django would need to provide some measure of validation and protection.
So - if this is a new feature, it isn't a v1.1 blocker. However, if you choose to look at this as a bug, then it's a bug with a known workaround ("don't do that"), that doesn't break any existing code, has no potential for data loss, and with no simple solution. To that end, it's still not a v1.1 blocker at this late stage in the development cycle.
comment:3 by , 15 years ago
If this is a new feature then existing behaviour should be documented.
"Captured parameters" section is right before "Defining URL Namespaces" and the fact that they do not work together as excepted is not mentioned.
I totally agree that fixing this ticket as-is is not a trivial task and it is not in 1.1 scope.
comment:4 by , 15 years ago
I didn't include my urlconfs, which showed multiple deployments of the same app in different areas of the site.
I can confirm however that the workaround mentioned works for me. I have a requirement to include some named groups in the urlconf of one deployment of the app, and another set in another deployment. I had tried to do this by having one named group in the root urls.py and another in the application urls.py. I then multiply-included the application urls.py (with varying named groups) in the root urlconf. This doesn't work with namespaces.
Workaround is to include a different urls.py for the app depending on the named groups required. Not as elegant, but it works.
comment:5 by , 15 years ago
Cc: | added |
---|
comment:6 by , 15 years ago
Cc: | added |
---|
by , 15 years ago
Attachment: | urlresolvers.reverse.hack.patch added |
---|
Hack to get parameterised parent namespace urlconfs to reverse (against 1.1)
comment:7 by , 15 years ago
Cc: | added |
---|---|
Patch needs improvement: | set |
In case anyone is interested in a hack -- not a proper solution -- I've attached a patch (against 1.1). Playing around in urlresolvers.py left me almost as clueless as before about its mechanics, so I'm convinced I'm missing edge (or non-edge!) cases. I needed this to multiply up a number of app URLs with various (parameterised) prefixes, giving them different and predictable names for reversing, but found later namespaces didn't support that (either) -- something to do with only one urlconf being searched per namespace unless I misunderstood something. Thus, I abandoned them and wrote a custom url include function instead. That means this hack has hardly seen any use.
by , 15 years ago
Attachment: | urlresolvers.reverse.hack.diff added |
---|
Maybe SVN diff against trunk with .diff extension then.. (Patch not tested on trunk)
comment:8 by , 15 years ago
Cc: | added |
---|
comment:9 by , 15 years ago
Cc: | added |
---|
comment:10 by , 15 years ago
I would argue in the opposite direction; that this is not a feature but is in fact a bug. Since forward URL resolution works completely in this regard it should be expected that the reverse() method does the same.
Because this is outside of the problem domain of the admin does not mean it's an invalid use case. I think extending the problem to 'provide some measure of validation and protection' is adding more to this issue that necessary. Those of us advanced folks creating modular apps that DRY out a repeated system or task know that the views in this particular app take a parameter or kwargs and per the django norm are expected to write views correctly. Django already just allows python to handle it by raising an exception so in that regard there is no difference.
comment:11 by , 15 years ago
Cc: | added |
---|
comment:12 by , 15 years ago
Cc: | added |
---|
comment:13 by , 14 years ago
Cc: | added |
---|
comment:14 by , 14 years ago
Cc: | added |
---|
comment:15 by , 14 years ago
Cc: | added |
---|
comment:16 by , 14 years ago
Cc: | added |
---|
comment:17 by , 14 years ago
Curious if there's been any further discussion with this change. I've been trying to deploy an instance of the admin UI with a dynamic prefix (basically, hosting instances of a webapp grouped by some identifier), and have a custom subclass of AdminSite that can take the parameters and do the right thing.
Now, this works, except that reversing admin URLs fails because of the way namespaces and captured arguments in the prefix of the URL works. This behavior has actually delayed our launch by about a week, so I'm eager to find a solution of some sort. I think where things seem confusing is that namespaced URLs just do not work the way standard URLs do.
I played around with a couple patch ideas. One thing off the top of my head would be to have the namespaces dictionary build up the possible matches in place of the raw prefix. In a sense, it'd act like reverse_dict, except keyed off by namespace. Then the right possibilities could be received based on namespace.
The trick at that point would be to pass that information to RegexURLResolver.reverse. Maybe the namespace could be passed to it, or the list of possibilities to use instead of those in reverse_dict.
Or I may be on totally the wrong path. However, from the CC list and my own experiences, it doesn't seem that the current design is really sufficient for more advanced use, and I'd be happy to work on this but want to make sure I'm not about to waste any time by going in a direction you guys don't want to go in.
comment:18 by , 14 years ago
by , 14 years ago
Attachment: | 11559.patch added |
---|
Added some polish to patch from #12950. Includes test.
comment:19 by , 14 years ago
Has patch: | set |
---|---|
Patch needs improvement: | unset |
Thanks carljm for finding my dupe.
Took me a while to wrap my head around the issue once I noticed it. The patch I've submitted includes a test to demonstrate the problem.
My solution involves essentially copying namespaced resolver into a new root resolver. That way any parameters can be resolved as normal and not interfere with non-namespaced functionality.
This new version uses memoize to reduce whatever performance impact that copying might have.
Going to toggle patch flags, though won't be offended if patch is deemed unfit. :)
comment:20 by , 14 years ago
Cc: | added |
---|
comment:21 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:22 by , 14 years ago
Cc: | added |
---|
comment:23 by , 14 years ago
Cc: | added |
---|---|
Easy pickings: | unset |
comment:24 by , 14 years ago
Cc: | removed |
---|
comment:25 by , 14 years ago
I've come across this problem whilst trying to develop a fairly complicated e-commerce platform. I want to use nested URL namespaces because it seems more DRY and elegant, so it would be useful if this could be implemented.
comment:27 by , 14 years ago
Keywords: | dceu2011 added |
---|---|
Triage Stage: | Someday/Maybe → Accepted |
Working on adding a few more test cases for this to make it complete.
comment:28 by , 14 years ago
Owner: | changed from | to
---|
by , 14 years ago
Attachment: | ticket_11559.diff added |
---|
Split the specific tests to own method, also added some for resolving the subitems
comment:29 by , 14 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:30 by , 13 years ago
Triage Stage: | Ready for checkin → Accepted |
---|
comment:31 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Setting it back to Ready for checkin, anonymous.. log in if you have a solid reason :)
I also have exactly the same issue. Bit of a let-down as I upgraded to 1.1 ahead of time in order to use namespaced urls exactly in this way!
In my example I have a 'username' argument as part of the root urlconf.
'/users/(?P%3Cusername%3E%5Cw+)/wikitestslug'
If I include the username argument:
NoReverseMatch: Reverse for 'wiki-view' with arguments '()' and keyword arguments '{'username': 'bar', 'slug': 'testslug'}' not found.
Seems like a fairly nasty bug for 1.1? Especially as url namespaces are one of the headline features of 1.1.