Proxito V2 (#10044)

This is an initial usage of the new proxito implementation to serve files. It's under a feature flag, so it can be enabled per-project.

- We still need to adapt our 404 handler to make use of this new implementation (it could be simplified with #10061)
- We are still using our re paths to match the URLs, but we don't make use of the captured parameters, we use the unresolver for that. This also means that things like #2292 won't be solved till we do the whole migration.
-  There is a lot of repetition from the original get method, but some of it could be simplified with #10065.

Tests will pass once we have merged one private PR that we have pending.

> **Note**
> We don't support custom urlconfs yet, so we shouldn't enable it for those projects yet.

Author: stsewd

readthedocs/core/unresolver.py

@@ -58,7 +58,7 @@ def __init__(self, project, language, filename):
         self.filename = filename
 
 
-class UnresolvedPathError(UnresolverError):
+class InvalidPathForVersionedProjectError(UnresolverError):
     def __init__(self, project, path):
         self.project = project
         self.path = path
@@ -411,7 +411,10 @@ def _unresolve_path_with_parent_project(
             if response:
                 return response
 
-        raise UnresolvedPathError(project=parent_project, path=path)
+        raise InvalidPathForVersionedProjectError(
+            project=parent_project,
+            path=self._normalize_filename(path),
+        )
 
     @staticmethod
     def get_domain_from_host(host):

readthedocs/projects/models.py

@@ -1854,6 +1854,7 @@ def add_features(sender, **kwargs):
     DISABLE_PAGEVIEWS = "disable_pageviews"
     DISABLE_SPHINX_DOMAINS = "disable_sphinx_domains"
     RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header"
+    USE_UNRESOLVER_WITH_PROXITO = "use_unresolver_with_proxito"
 
     # Versions sync related features
     SKIP_SYNC_TAGS = 'skip_sync_tags'
@@ -1969,6 +1970,10 @@ def add_features(sender, **kwargs):
             RESOLVE_PROJECT_FROM_HEADER,
             _("Allow usage of the X-RTD-Slug header"),
         ),
+        (
+            USE_UNRESOLVER_WITH_PROXITO,
+            _("Use new unresolver implementation for serving documentation files."),
+        ),
 
         # Versions sync related features
         (

readthedocs/proxito/tests/test_full.py

@@ -364,6 +364,18 @@ def test_custom_domain_response_hsts(self):
         )
 
 
+class ProxitoV2TestFullDocServing(TestFullDocServing):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )
+
+
 class TestDocServingBackends(BaseDocServing):
     # Test that nginx and python backends both work
 
@@ -555,6 +567,18 @@ def test_track_downloads(self, is_audit_enabled):
         self.assertEqual(log.action, AuditLog.DOWNLOAD)
 
 
+class ProxitoV2TestDocServingBackends(TestDocServingBackends):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )
+
+
 @override_settings(
     PYTHON_MEDIA=False,
     PUBLIC_DOMAIN='readthedocs.io',
@@ -1333,6 +1357,18 @@ def test_serve_invalid_static_file(self, staticfiles_storage):
             self.assertEqual(resp.status_code, 404)
 
 
+class ProxitoV2TestAdditionalDocViews(TestAdditionalDocViews):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )
+
+
 @override_settings(
     ALLOW_PRIVATE_REPOS=True,
     PUBLIC_DOMAIN='dev.readthedocs.io',
@@ -1587,3 +1623,15 @@ def test_cache_on_plan(self):
         # Add project to plan, so we're using that to enable CDN
         self.organization.projects.add(self.project)
         self._test_cache_control_header_project(expected_value="public")
+
+
+class ProxitoV2TestCDNCache(TestCDNCache):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )

readthedocs/proxito/tests/test_headers.py

@@ -1,9 +1,10 @@
 import django_dynamic_fixture as fixture
 from django.test import override_settings
 from django.urls import reverse
+from django_dynamic_fixture import get
 
 from readthedocs.builds.constants import LATEST
-from readthedocs.projects.models import Domain, HTTPHeader
+from readthedocs.projects.models import Domain, Feature, HTTPHeader
 
 from .base import BaseDocServing
 
@@ -143,3 +144,15 @@ def test_cache_headers_public_version_with_private_projects_allowed(self):
         r = self.client.get('/en/latest/', HTTP_HOST='project.dev.readthedocs.io')
         self.assertEqual(r.status_code, 200)
         self.assertEqual(r["CDN-Cache-Control"], "public")
+
+
+class ProxitoV2HeaderTests(ProxitoHeaderTests):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )

readthedocs/proxito/tests/test_middleware.py

@@ -259,6 +259,18 @@ def test_front_slash_url(self):
         )
 
 
+class ProxitoV2MiddlewareTests(MiddlewareTests):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )
+
+
 @pytest.mark.proxito
 @override_settings(PUBLIC_DOMAIN='dev.readthedocs.io')
 class MiddlewareURLConfTests(TestCase):

readthedocs/proxito/tests/test_old_redirects.py

@@ -14,6 +14,7 @@
 from django.http import Http404
 from django.test.utils import override_settings
 from django.urls import Resolver404
+from django_dynamic_fixture import get
 
 from readthedocs.builds.models import Version
 from readthedocs.projects.models import Feature
@@ -153,6 +154,18 @@ def test_root_redirect_with_query_params(self):
         )
 
 
+class ProxitoV2InternalRedirectTests(InternalRedirectTests):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )
+
+
 # Use ``PYTHON_MEDIA`` here to raise a 404 when trying to serve the file
 # from disk and execute the code for the handler404 (the file does not
 # exist). On production, this will happen when trying to serve from
@@ -649,6 +662,18 @@ def test_not_found_page_without_trailing_slash(self):
             )
 
 
+class ProxitoV2UserRedirectTests(UserRedirectTests):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )
+
+
 @override_settings(PUBLIC_DOMAIN="dev.readthedocs.io")
 class UserForcedRedirectTests(BaseDocServing):
     def test_no_forced_redirect(self):
@@ -943,6 +968,19 @@ def test_redirect_with_301_status(self):
             "http://project.dev.readthedocs.io/en/latest/tutorial/install.html",
         )
 
+
+class ProxitoV2UserForcedRedirectTests(UserForcedRedirectTests):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )
+
+
 @override_settings(
     PYTHON_MEDIA=True,
     PUBLIC_DOMAIN="dev.readthedocs.io",
@@ -1085,3 +1123,15 @@ def test_redirect_sphinx_html_crossdomain(self):
             )
             self.assertEqual(r.status_code, 302, url)
             self.assertEqual(r["Location"], expected_location, url)
+
+
+class ProxitoV2UserRedirectCrossdomainTest(UserRedirectCrossdomainTest):
+    # TODO: remove this class once the new implementation is the default.
+    def setUp(self):
+        super().setUp()
+        get(
+            Feature,
+            feature_id=Feature.USE_UNRESOLVER_WITH_PROXITO,
+            default_true=True,
+            future_default_true=True,
+        )