Opened 10 years ago
Closed 7 years ago
#23977 closed New feature (fixed)
Add setdefault() to HttpResponse
Reported by: | Robert Coup | Owned by: | Sergey |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Дилян Палаузов | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
HttpResponse acts like a dictionary with respect to headers. It's a common middleware pattern to set headers unless they've been explicitly set by the view. eg.
class MyMiddleware(object): def process_response(self, request, response): if not 'X-Test' in response: response['X-Test'] = 12345 return response
Having a setdefault()
implementation would simplify this (not much in the trivial one-header case, but for example CORS middleware where you're setting a number of headers)
class MyMiddleware(object): def process_response(self, request, response): response.setdefault('X-Test', 12345) return response
Change History (6)
comment:1 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 10 years ago
Has patch: | set |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:4 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:5 by , 7 years ago
Cc: | added |
---|---|
Resolution: | fixed |
Status: | closed → new |
HttpResponseBase.setdefault should return a value in order to be consistent with dict.setdefault and what users expect from a function called setdefault on an objects, that behaves like dict.
The method should also be utilized in the existing code:
diff --git a/django/middleware/common.py b/django/middleware/common.py --- a/django/middleware/common.py +++ b/django/middleware/common.py @@ -107,8 +107,8 @@ class CommonMiddleware(MiddlewareMixin): # Add the Content-Length header to non-streaming responses if not # already set. - if not response.streaming and not response.has_header('Content-Length'): - response['Content-Length'] = str(len(response.content)) + if not response.streaming: + response.setdefault('Content-Length', str(len(response.content))) return response diff --git a/django/utils/cache.py b/django/utils/cache.py --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -246,8 +246,7 @@ def patch_response_headers(response, cache_timeout=None): cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS if cache_timeout < 0: cache_timeout = 0 # Can't have max-age negative - if not response.has_header('Expires'): - response['Expires'] = http_date(time.time() + cache_timeout) + response.setdefault('Expires', http_date(time.time() + cache_timeout)) patch_cache_control(response, max_age=cache_timeout) diff --git a/django/views/decorators/http.py b/django/views/decorators/http.py --- a/django/views/decorators/http.py +++ b/django/views/decorators/http.py @@ -101,10 +101,10 @@ def condition(etag_func=None, last_modified_func=None): # Set relevant headers on the response if they don't already exist # and if the request method is safe. if request.method in ('GET', 'HEAD'): - if res_last_modified and not response.has_header('Last-Modified'): - response['Last-Modified'] = http_date(res_last_modified) - if res_etag and not response.has_header('ETag'): - response['ETag'] = res_etag + if res_last_modified:: + response.setdefault('Last-Modified', http_date(res_last_modified)) + if res_etag: + response.setdefault('ETag', res_etag) return response
comment:6 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Please open a new ticket rather then reopening an existing one that's been fixed for three years. You can provide patches as attachments rather than as comments.
In 059c9ab24c41e1460fd8b7af65ea8d5f80f1aa82: