Opened 11 years ago
Closed 8 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 , 11 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 11 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 11 years ago
| Has patch: | set |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
comment:4 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:5 by , 8 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 , 8 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: