[Django] #25409: Allow url and groups of urls to be easily tagged and selected
Django
---------------------------------+--------------------
Reporter: atul-bhouraskar | Owner: nobody
Type: New feature | Status: new
Component: Core (URLs) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
---------------------------------+--------------------
== The Problem ==
Often (usually in middleware) processing has to be applied to a certain
URLs only eg CORS.
The usual way to specify this would be to create an additional set of
regex patterns identifying these urls - eg.
{{{
CORS_URLS_REGEX = r'^/api/.*$'
}}}
The middleware then typically matches the incoming request URL to the
regex and determines whether it is to be selected for processing.
This approach has several limitations including:
* It violates DRY as the regexes in the settings have to be synced with
the actual URL patterns
* Matching multiple patterns may require the app writer to essentially
reinvent URL patterns - poorly.
== The Proposal ==
Add an optional {{{tags}}} keyword argument to {{{django.conf.urls.url}}}
allowing a URL to be optionally tagged with one or more tags which can
then be retrieved via {{{HttpRequest.resolver_match.tags}}} in the
middleware / view etc.
{{{
urlpatterns = [
url(r'^$', views.home, name='home'),
url(r'^articles/$', views.show_articles, name='show-articles',
tags=['require_GET']),
url(r'^private/$', include(private_patterns),
tags=['login_required']),
url(r'^api/', include(api_patterns), tags=[
'cors_allowed', 'nologin_forbidden', 'ajax_required'
]),
]
}}}
In the example above, the {{{home}}} url has no tags, the {{{show-
articles}}} url is tagged {{{require_GET}}}, the urls under {{{private/}}}
are all tagged {{{login_required}}} while the the urls under {{{api/}}}
have multiple tags.
This allows app middleware to selectively process urls very easily:
{{{
class LoginRequiredMiddleware(object):
def process_view(self, request, view_func, view_args, view_kwargs):
if 'login_required' in request.resolver_match.tags and \
not request.user.is_authenticated():
return redirect('login')
class RequireGETMiddleware(object):
def process_view(self, request, view_func, view_args, view_kwargs):
if 'require_GET' in request.resolver_match.tags and request.method
!= 'GET':
return HttpResponseNotAllowed(['GET'])
class AjaxRequiredMiddleware(object):
def process_view(self, request, view_func, view_args, view_kwargs):
if 'ajax_required' in request.resolver_match.tags and not
request.is_ajax():
return HttpResponseForbidden()
class CorsMiddleware(object):
def process_view(self, request, view_func, view_args, view_kwargs):
if 'cors_allowed' in request.resolver_match.tags:
# continue CORS processing
}}}
I am attaching a patch that implements this tagging feature to
urlpatterns. It has tests (docs to be added). The change is fully
backwards compatible as specifying tags is completely optional, all
existing tests pass with the patch applied to master.
Comments welcome!
--
Ticket URL: <https://code.djangoproject.com/ticket/25409>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: atul-bhouraskar | Owner: nobody
Type: New feature | Status: new
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
Changes (by atul-bhouraskar):
* Attachment "urltags.patch" added.
Patch implementing url tagging.
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Component: Core (URLs) | Version: master
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by atul-bhouraskar):
* status: new => assigned
* needs_better_patch: => 0
* needs_tests: => 0
* owner: nobody => atul-bhouraskar
* needs_docs: => 0
Old description:
New description:
I'm working on my branch at https://github.com/atul-
bhouraskar/django/tree/ticket_25409
Comments welcome!
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:1>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by timgraham):
* stage: Unreviewed => Someday/Maybe
Comment:
Hi, there is major work planned for URLs in 1.10. See the
[https://groups.google.com/d/topic/django-
developers/9AqFxIIW2Mk/discussion django-developers] discussion. I am not
sure if a similar concept might be part of that work, but we'll probably
want to wait until that work is completed and merged before proceeding
with this idea.
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:2>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by atul-bhouraskar):
Hi Tim,
Thanks for looking at this. I've looked the URLs work planned for 1.10 but
that has a different focus - of making it easy to extend the URL resolver
and is proposing a new internal architecture. That is orthogonal to what
we are talking about here.
Here we are not touching the core of the resolver but simply adding a
mechanism to pass on tags from the pattern to the request via the
resolver. The actual code changes are in 10 code lines most of which are
simple assignments as the tags get passed to the final match object. The
rest are tests. It would be easy to add it to the new architecture at any
time.
Other than documentation (which I can add in pretty quickly) the patch can
be easily merged in (unless there are design issues that need to be
discussed which I am fine with).
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:3>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by timgraham):
Yes, it would be a good idea to raise the issue on the
DevelopersMailingList for a design review. The main reason for waiting to
merge until after the major work is completed is to avoid creating merge
conflicts with that branch as a lot of code and documentation is being
moved.
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:4>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by knbk):
Hi, the major rework for URLs involves a different approach to the same
problem. Instead of "tagging" urls and filtering for tags in the
middleware, it allows you to apply decorators to a group of urls, i.e.:
{{{
from ... import login_required
urlpatterns = [
...
url(r'^private/$', include(private_patterns),
decorators=[login_required]),
]
}}}
This will apply the `login_required` decorator to each view in
`private_patterns`. Most middleware that might be applied to a certain
section of a site already has matching decorators, so this should give you
pretty much the same options.
If you'd like to discuss your approach as an alternative, please raise the
issue on the mailing list.
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:5>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by @…):
* cc: barfieldsamueliii10@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:6>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by atul-bhouraskar):
* cc: atul-bhouraskar (added)
Comment:
Thanks! I'll create a discussion topic on the mailing list.
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:7>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by atul-bhouraskar):
Created pull request to help with design discussion
https://github.com/django/django/pull/5309
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:8>
Django
Reporter: atul-bhouraskar | Owner: @…
Type: New feature | Status: assigned
Keywords: BARFIELD III, | Triage Stage:
SAMUEL | Someday/Maybe
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by @…):
* needs_tests: 0 => 1
* owner: atul-bhouraskar => @…
* easy: 0 => 1
* keywords: => BARFIELD III, SAMUEL
* needs_docs: 0 => 1
* ui_ux: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:9>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by timgraham):
[https://groups.google.com/d/topic/django-
developers/Y6pJyqOmllA/discussion django-dev discussion].
[https://github.com/django/django/pull/6141 PR] for another proposal:
`url(r'^$', include('suburl'), wrap=[wrap1, wrap2])`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:9>
Django
Reporter: atul-bhouraskar | Owner: atul-
| bhouraskar
Type: New feature | Status: assigned
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by danihodovic):
Was this feature ever implemented? I'm looking for a way to disable
caching for all admin urls.
--
Ticket URL: <https://code.djangoproject.com/ticket/25409#comment:10>