api_route: Ensure never_cache is honored for all response status codes

Previously never cache headers were only added for responses with status code
200 but error responses must also be considered.

So move never_cache parameter handling of api_route decorator from apiurls
module to apiresponse one.

Related to T2774

swh/web/api/apiresponse.py

@@ -12,6 +12,7 @@ import sentry_sdk
 
 from django.http import HttpResponse
 from django.shortcuts import render
+from django.utils.cache import add_never_cache_headers
 from django.utils.html import escape
 from rest_framework.exceptions import APIException
 from rest_framework.request import Request
@@ -151,20 +152,25 @@ def make_api_response(
             if not doc_data["noargs"]:
                 doc_data["endpoint_path"][-1]["path"] += "/doc/"
 
-        return render(
+        response = render(
             request, "api/apidoc.html", doc_data, status=doc_data["status_code"]
         )
 
     # otherwise simply return the raw data and let DRF picks
     # the correct renderer (JSON or YAML)
     else:
-        return Response(
+        response = Response(
             data,
             headers=headers,
             content_type=request.accepted_media_type,
             status=doc_data["status_code"],
         )
 
+    if getattr(request, "never_cache", False):
+        add_never_cache_headers(response)
+
+    return response
+
 
 def error_response(
     request: Request, exception: Exception, doc_data: Dict[str, Any]

swh/web/api/apiurls.py

@@ -7,7 +7,6 @@ import functools
 from typing import Dict, List, Optional
 
 from django.http.response import HttpResponseBase
-from django.utils.cache import add_never_cache_headers
 from rest_framework.decorators import api_view
 
 from swh.web.api import throttling
@@ -92,6 +91,8 @@ def api_route(
         @throttling.throttle_scope(throttle_scope)
         @functools.wraps(f)
         def api_view_f(request, **kwargs):
+            # never_cache will be handled in apiresponse module
+            request.never_cache = never_cache
             response = f(request, **kwargs)
             doc_data = None
             # check if response has been forwarded by api_doc decorator
@@ -106,9 +107,6 @@ def api_route(
             else:
                 api_response = response
 
-            if never_cache:
-                add_never_cache_headers(api_response)
-
             return api_response
 
         # small hacks for correctly generating API endpoints index doc

swh/web/tests/api/test_apiurls.py

@@ -23,6 +23,15 @@ def api_never_cache_route(request, int_arg):
     return {"result": int(int_arg)}
 
 
+@api_route(
+    r"/never/cache/route/error/",
+    "api-1-never-cache-route-with-error",
+    never_cache=True,
+)
+def api_never_cache_route_with_error(request):
+    raise Exception("error")
+
+
 def test_api_route_with_cache(api_client):
     url = reverse("api-1-some-route", url_args={"int_arg": 1})
     resp = check_api_get_responses(api_client, url, status_code=200)
@@ -30,9 +39,19 @@ def test_api_route_with_cache(api_client):
     assert "Cache-Control" not in resp
 
 
+_cache_control = "max-age=0, no-cache, no-store, must-revalidate"
+
+
 def test_api_route_never_cache(api_client):
     url = reverse("api-1-never-cache-route", url_args={"int_arg": 1})
     resp = check_api_get_responses(api_client, url, status_code=200)
     assert resp.data == {"result": 1}
     assert "Cache-Control" in resp
-    assert resp["Cache-Control"] == "max-age=0, no-cache, no-store, must-revalidate"
+    assert resp["Cache-Control"] == _cache_control
+
+
+def test_api_route_never_cache_with_error(api_client):
+    url = reverse("api-1-never-cache-route-with-error")
+    resp = check_api_get_responses(api_client, url, status_code=500)
+    assert "Cache-Control" in resp
+    assert resp["Cache-Control"] == _cache_control

[Phabricator Migration user]

Some references in the commit message have been migrated:

• T2774 is now swh/infra/sysadm-environment#2774 (closed)