Add utility method for explicitly setting cache tags to cache tag class

readthedocs/api/mixins.py

@@ -17,11 +17,8 @@ class CDNCacheTagsMixin:
     """
     Add cache tags for project and version to the response of this view.
 
-    The view inheriting this mixin should implement the
-    `self._get_project` and `self._get_version` methods.
-
-    If `self._get_version` returns `None`,
-    only the project level tags are added.
+    The view inheriting this mixin can either call :py:method:`set_cache_tags` or
+    implement the ``self._get_project`` and ``self._get_version`` methods.
 
     You can add an extra per-project tag by overriding the `project_cache_tag` attribute.
     """
@@ -30,29 +27,40 @@ class CDNCacheTagsMixin:
 
     def dispatch(self, request, *args, **kwargs):
         response = super().dispatch(request, *args, **kwargs)
-        cache_tags = self._get_cache_tags()
+        cache_tags = getattr(self, "_cache_tags", self._get_cache_tags())
         if cache_tags:
             add_cache_tags(response, cache_tags)
         return response
 
-    def _get_cache_tags(self):
+    def _get_cache_tags(self, project=None, version=None):
         """
         Get cache tags for this view.
 
+        This returns an array of tag identifiers used to tag the response at CDN.
+
+        If project and version are not passed in, these values will come from the
+        methods ``_get_project()`` and ``_get_version()``.
+        If ``_get_version()`` returns ``None``, only the project level tags are added.
+
+        It's easier to use :py:method:`set_cache_tags` if project/version aren't
+        set at the instance level, or if they are passed in through a method
+        like ``get()``.
+
         .. warning::
 
            This method is run at the end of the request,
            so any exceptions like 404 should be caught.
         """
-        try:
-            project = self._get_project()
-            version = self._get_version()
-        except Exception:
-            log.warning(
-                "Error while retrieving project or version for this view.",
-                exc_info=True,
-            )
-            return []
+        if project is None and version is None:
+            try:
+                project = self._get_project()
+                version = self._get_version()
+            except Exception:
+                log.warning(
+                    "Error while retrieving project or version for this view.",
+                    exc_info=True,
+                )
+                return []
 
         tags = []
         if project:
@@ -63,6 +71,19 @@ def _get_cache_tags(self):
             tags.append(get_cache_tag(project.slug, self.project_cache_tag))
         return tags
 
+    def set_cache_tags(self, project=None, version=None):
+        """
+        Store cache tags to be added to response.
+
+        This method can be used if project/version do not exist on the view
+        instance or if they are passed into the view through a method like
+        ``get()``.
+
+        The attribute methods ``_get_project()``/``_get_version()`` aren`t used
+        in this pattern.
+        """
+        self._cache_tags = self._get_cache_tags(project, version)
+
 
 class EmbedAPIMixin:
     """

readthedocs/projects/views/public.py

@@ -382,12 +382,7 @@ def get(
                 slug=version_slug,
             )
 
-        # TODO don't do this, it's a leftover of trying to use CDNCacheTagsMixin
-        # without class level variables. See proxito.views.serve for
-        # other instances of this pattern to update.
-        # See: https://github.com/readthedocs/readthedocs.org/pull/12495
-        self.project = version.project
-        self.version = version
+        self.set_cache_tags(project=version.project, version=version)
 
         return self._serve_dowload(
             request=request,

readthedocs/proxito/tests/test_full.py

@@ -857,6 +857,7 @@ def test_default_robots_txt(self, storage_exists):
             reverse("robots_txt"), headers={"host": "project.readthedocs.io"}
         )
         self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.headers["Cache-Tag"], "project,project:robots.txt")
         expected = dedent(
             """
             User-agent: *
@@ -909,6 +910,7 @@ def test_default_robots_txt_disallow_hidden_versions(self, storage_exists):
             reverse("robots_txt"), headers={"host": "project.readthedocs.io"}
         )
         self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.headers["Cache-Tag"], "project,project:robots.txt")
         expected = dedent(
             """
             User-agent: *
@@ -932,6 +934,7 @@ def test_default_robots_txt_private_version(self, storage_exists):
             reverse("robots_txt"), headers={"host": "project.readthedocs.io"}
         )
         self.assertEqual(response.status_code, 404)
+        self.assertNotIn("Cache-Tag", response.headers)
 
     def test_custom_robots_txt(self):
         self.project.versions.update(active=True, built=True)
@@ -942,6 +945,7 @@ def test_custom_robots_txt(self):
             response["x-accel-redirect"],
             "/proxito/media/html/project/latest/robots.txt",
         )
+        self.assertEqual(response.headers["Cache-Tag"], "project,project:latest,project:robots.txt")
 
     def test_custom_robots_txt_private_version(self):
         self.project.versions.update(
@@ -951,6 +955,7 @@ def test_custom_robots_txt_private_version(self):
             reverse("robots_txt"), headers={"host": "project.readthedocs.io"}
         )
         self.assertEqual(response.status_code, 404)
+        self.assertNotIn("Cache-Tag", response.headers)
 
     def test_directory_indexes(self):
         self.project.versions.update(active=True, built=True)

readthedocs/proxito/views/serve.py

@@ -640,6 +640,10 @@ def get(self, request):
         """
         project = request.unresolved_domain.project
 
+        # Set cache tag only for project for now, for responses that don't use a
+        # specific version instance.
+        self.set_cache_tags(project=project)
+
         if project.delisted:
             return render(
                 request,
@@ -690,6 +694,9 @@ def get(self, request):
                 filename="robots.txt",
                 check_if_exists=True,
             )
+            # Set project and version cache tags, override project only tag as
+            # we want to purge this file each time the version changes too.
+            self.set_cache_tags(project=project, version=version)
             log.info("Serving custom robots.txt file.")
             return response
         except StorageFileNotFound:
@@ -704,6 +711,7 @@ def get(self, request):
             "sitemap_url": sitemap_url,
             "hidden_paths": self._get_hidden_paths(project),
         }
+
         return render(
             request,
             "robots.txt",
@@ -720,17 +728,6 @@ def _get_hidden_paths(self, project):
         ]
         return hidden_paths
 
-    def _get_project(self):
-        # Method used by the CDNCacheTagsMixin class.
-        return self.request.unresolved_domain.project
-
-    def _get_version(self):
-        # Method used by the CDNCacheTagsMixin class.
-        # This view isn't explicitly mapped to a version,
-        # but it can be when we serve a custom robots.txt file.
-        # TODO: refactor how we set cache tags to avoid this.
-        return None
-
 
 class ServeRobotsTXT(SettingsOverrideObject):
     _default_class = ServeRobotsTXTBase
@@ -877,6 +874,9 @@ def changefreqs_generator():
 
             versions.append(element)
 
+        # Cache tag only for project, don't include version cache tag
+        self.set_cache_tags(project=project)
+
         context = {
             "versions": versions,
         }
@@ -887,16 +887,6 @@ def changefreqs_generator():
             content_type="application/xml",
         )
 
-    def _get_project(self):
-        # Method used by the CDNCacheTagsMixin class.
-        return self.request.unresolved_domain.project
-
-    def _get_version(self):
-        # Method used by the CDNCacheTagsMixin class.
-        # This view isn't explicitly mapped to a version,
-        # TODO: refactor how we set cache tags to avoid this.
-        return None
-
 
 class ServeSitemapXML(SettingsOverrideObject):
     _default_class = ServeSitemapXMLBase
@@ -917,25 +907,20 @@ class ServeStaticFiles(CDNCacheControlMixin, CDNCacheTagsMixin, ServeDocsMixin,
 
     def get(self, request, filename):
         try:
-            return self._serve_static_file(request=request, filename=filename)
+            resp = self._serve_static_file(request=request, filename=filename)
+            # Cache tag only for project, don't include version cache tag
+            self.set_cache_tags(project=request.unresolved_domain.project)
+            return resp
         except InvalidPathError:
             raise Http404
 
-    def _get_cache_tags(self):
+    def _get_cache_tags(self, project=None, version=None):
         """
         Add an additional *global* tag.
 
         This is so we can purge all files from all projects
         with one single call.
         """
-        tags = super()._get_cache_tags()
+        tags = super()._get_cache_tags(project=project, version=version)
         tags.append(self.project_cache_tag)
         return tags
-
-    def _get_project(self):
-        # Method used by the CDNCacheTagsMixin class.
-        return self.request.unresolved_domain.project
-
-    def _get_version(self):
-        # This view isn't attached to a version.
-        return None