Opened 3 years ago

Closed 13 days ago

#23977 closed New feature (fixed)

Add setdefault() to HttpResponse

Reported by: Robert Coup Owned by: Sergey
Component: HTTP handling Version: master
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 Changed 3 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:2 Changed 3 years ago by Sergey

Owner: changed from nobody to Sergey
Status: newassigned

comment:3 Changed 3 years ago by Tim Graham

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:4 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 059c9ab24c41e1460fd8b7af65ea8d5f80f1aa82:

Fixed #23977 -- Added setdefault() method to HttpResponse

comment:5 Changed 13 days ago by Дилян Палаузов

Cc: Дилян Палаузов added
Resolution: fixed
Status: closednew

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
 
Last edited 13 days ago by Дилян Палаузов (previous) (diff)

comment:6 Changed 13 days ago by Tim Graham

Resolution: fixed
Status: newclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top