Proxito: actually cache robots.txt and sitemap.xml (#10123)

Forgot to actually inherit from the CDNCacheControlMixin,
so caching for these views isn't actually working on .com.

Author: stsewd

readthedocs/proxito/tests/base.py

@@ -33,7 +33,7 @@ def setUp(self):
             users=[self.eric],
             main_language_project=None,
         )
-        self.project.versions.update(privacy_level=PUBLIC)
+        self.project.versions.update(privacy_level=PUBLIC, built=True, active=True)
         self.version = self.project.versions.get(slug=LATEST)
 
         self.subproject = fixture.get(

readthedocs/proxito/tests/test_headers.py

@@ -145,6 +145,34 @@ def test_cache_headers_public_version_with_private_projects_allowed(self):
         self.assertEqual(r.status_code, 200)
         self.assertEqual(r["CDN-Cache-Control"], "public")
 
+    @override_settings(ALLOW_PRIVATE_REPOS=False)
+    def test_cache_headers_robots_txt_with_private_projects_not_allowed(self):
+        r = self.client.get("/robots.txt", HTTP_HOST="project.dev.readthedocs.io")
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r["CDN-Cache-Control"], "public")
+        self.assertEqual(r["Cache-Tag"], "project,project:robots.txt")
+
+    @override_settings(ALLOW_PRIVATE_REPOS=True)
+    def test_cache_headers_robots_txt_with_private_projects_allowed(self):
+        r = self.client.get("/robots.txt", HTTP_HOST="project.dev.readthedocs.io")
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r["CDN-Cache-Control"], "public")
+        self.assertEqual(r["Cache-Tag"], "project,project:robots.txt")
+
+    @override_settings(ALLOW_PRIVATE_REPOS=False)
+    def test_cache_headers_robots_txt_with_private_projects_not_allowed(self):
+        r = self.client.get("/sitemap.xml", HTTP_HOST="project.dev.readthedocs.io")
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r["CDN-Cache-Control"], "public")
+        self.assertEqual(r["Cache-Tag"], "project,project:sitemap.xml")
+
+    @override_settings(ALLOW_PRIVATE_REPOS=True)
+    def test_cache_headers_robots_txt_with_private_projects_allowed(self):
+        r = self.client.get("/sitemap.xml", HTTP_HOST="project.dev.readthedocs.io")
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r["CDN-Cache-Control"], "public")
+        self.assertEqual(r["Cache-Tag"], "project,project:sitemap.xml")
+
 
 class ProxitoV2HeaderTests(ProxitoHeaderTests):
     # TODO: remove this class once the new implementation is the default.

readthedocs/proxito/views/serve.py

@@ -42,7 +42,19 @@
 
 
 class ServePageRedirect(CDNCacheControlMixin, ServeRedirectMixin, ServeDocsMixin, View):
+
+    """
+    Page redirect view.
+
+    This allows users to redirec to the default version of a project.
+    For example:
+
+    - /page/api/index.html -> /en/latest/api/index.html
+    - /projects/subproject/page/index.html -> /projects/subproject/en/latest/api/index.html
+    """
+
     def get(self, request, subproject_slug=None, filename=""):
+        """Handle all page redirects."""
 
         unresolved_domain = request.unresolved_domain
         project = unresolved_domain.project
@@ -73,6 +85,14 @@ def get(self, request, subproject_slug=None, filename=""):
 
 
 class ServeDocsBase(CDNCacheControlMixin, ServeRedirectMixin, ServeDocsMixin, View):
+
+    """
+    Serve docs view.
+
+    This view serves all the documentation pages,
+    and handles canonical redirects.
+    """
+
     def get(
         self,
         request,
@@ -89,6 +109,7 @@ def get(
         ``subproject_slash`` is used to determine if the subproject URL has a slash,
         so that we can decide if we need to serve docs or add a /.
         """
+        # pylint: disable=too-many-locals
         unresolved_domain = request.unresolved_domain
         # Handle requests that need canonicalizing first,
         # e.g. HTTP -> HTTPS, redirect to canonical domain, etc.
@@ -282,6 +303,7 @@ def _get_canonical_redirect_type(self, request):
                 .exists()
             )
             # For .com we need to check if the project supports custom domains.
+            # pylint: disable=protected-access
             if canonical_domain and resolver._use_cname(project):
                 log.debug(
                     "Proxito Public Domain -> Canonical Domain Redirect.",
@@ -442,9 +464,15 @@ class ServeDocs(SettingsOverrideObject):
     _default_class = ServeDocsBase
 
 
-class ServeError404Base(ServeRedirectMixin, ServeDocsMixin, View):
+class ServeError404Base(CDNCacheControlMixin, ServeRedirectMixin, ServeDocsMixin, View):
+
+    """
+    Proxito handler for 404 pages.
 
-    def get(self, request, proxito_path, template_name='404.html'):
+    This view is called by an internal nginx redirect when there is a 404.
+    """
+
+    def get(self, request, proxito_path):
         """
         Handler for 404 pages on subdomains.
 
@@ -460,6 +488,7 @@ def get(self, request, proxito_path, template_name='404.html'):
         with the default version and finally, if none of them are found, the Read
         the Docs default page (Maze Found) is rendered by Django and served.
         """
+        # pylint: disable=too-many-locals
         log.bind(proxito_path=proxito_path)
         log.debug('Executing 404 handler.')
 
@@ -798,10 +827,14 @@ class ServeError404(SettingsOverrideObject):
     _default_class = ServeError404Base
 
 
-class ServeRobotsTXTBase(ServeDocsMixin, View):
+class ServeRobotsTXTBase(CDNCacheControlMixin, CDNCacheTagsMixin, ServeDocsMixin, View):
+
+    """Serve robots.txt from the domain's root."""
 
     # Always cache this view, since it's the same for all users.
     cache_response = True
+    # Extra cache tag to invalidate only this view if needed.
+    project_cache_tag = "robots.txt"
 
     def get(self, request):
         """
@@ -822,7 +855,7 @@ def get(self, request):
             )
 
         # Verify if the project is marked as spam and return a custom robots.txt
-        elif "readthedocsext.spamfighting" in settings.INSTALLED_APPS:
+        if "readthedocsext.spamfighting" in settings.INSTALLED_APPS:
             from readthedocsext.spamfighting.utils import is_robotstxt_denied  # noqa
             if is_robotstxt_denied(project):
                 return render(
@@ -894,15 +927,30 @@ 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
 
 
-class ServeSitemapXMLBase(View):
+class ServeSitemapXMLBase(CDNCacheControlMixin, CDNCacheTagsMixin, View):
+
+    """Serve sitemap.xml from the domain's root."""
 
     # Always cache this view, since it's the same for all users.
     cache_response = True
+    # Extra cache tag to invalidate only this view if needed.
+    project_cache_tag = "sitemap.xml"
 
     def get(self, request):
         """
@@ -1034,6 +1082,16 @@ 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