Merge pull request #6867 from readthedocs/optimize-resolve-path

Optimize resolve_path

Author: stsewd

readthedocs/core/resolver.py

@@ -99,34 +99,18 @@ def resolve_path(
             cname=None,
     ):
         """Resolve a URL with a subset of fields defined."""
-        cname = cname or project.get_canonical_custom_domain()
         version_slug = version_slug or project.get_default_version()
         language = language or project.language
 
         filename = self._fix_filename(project, filename)
 
-        current_project = project
-        project_slug = project.slug
-        subproject_slug = None
-        # We currently support more than 2 levels of nesting subprojects and
-        # translations, only loop twice to avoid sticking in the loop
-        for _ in range(0, 2):
-            main_language_project = current_project.main_language_project
-            relation = current_project.get_parent_relationship()
-
-            if main_language_project:
-                current_project = main_language_project
-                project_slug = main_language_project.slug
-                language = project.language
-                subproject_slug = None
-            elif relation:
-                current_project = relation.parent
-                project_slug = relation.parent.slug
-                subproject_slug = relation.alias
-                cname = relation.parent.domains.filter(canonical=True).first()
-            else:
-                break
-
+        main_project, subproject_slug = self._get_canonical_project_data(project)
+        project_slug = main_project.slug
+        cname = (
+            cname
+            or self._use_subdomain()
+            or main_project.get_canonical_custom_domain()
+        )
         single_version = bool(project.single_version or single_version)
 
         return self.base_resolve_path(
@@ -195,6 +179,68 @@ def resolve(
         )
         return urlunparse((protocol, domain, path, '', query_params, ''))
 
+    def _get_canonical_project_data(self, project):
+        """
+        Returns a tuple with (project, subproject_slug) from the canonical project of `project`.
+
+        We currently support more than 2 levels of nesting subprojects and translations,
+        but we only serve 2 levels to avoid sticking in the loop.
+        This means, we can have the following cases:
+
+        - The project isn't a translation or subproject
+
+          We serve the documentation from the domain of the project itself
+          (main.docs.com/).
+
+        - The project is a translation of a project
+
+          We serve the documentation from the domain of the main translation
+          (main.docs.com/es/).
+
+        - The project is a subproject of a project
+
+          We serve the documentation from the domain of the super project
+          (main.docs.com/projects/subproject/).
+
+        - The project is a translation, and the main translation is a subproject of a project, like:
+
+          - docs
+          - api (subproject of ``docs``)
+          - api-es (translation of ``api``, and current project to be served)
+
+          We serve the documentation from the domain of the super project
+          (docs.docs.com/projects/api/es/).
+
+        - The project is a subproject, and the superproject is a translation of a project, like:
+
+          - docs
+          - docs-es (translation of ``docs``)
+          - api-es (subproject of ``docs-es``, and current project to be served)
+
+          We serve the documentation from the domain of the super project (the translation),
+          this is docs-es.docs.com/projects/api-es/es/.
+          We aren't going to support this case for now.
+
+        In summary: If the project is a subproject,
+        we don't care if the superproject is a translation,
+        we always serve from the domain of the superproject.
+        If the project is a translation,
+        we need to check if the main translation is a subproject.
+        """
+        main_project = project
+        subproject_slug = None
+
+        main_language_project = main_project.main_language_project
+        if main_language_project:
+            main_project = main_language_project
+
+        relation = main_project.get_parent_relationship()
+        if relation:
+            main_project = relation.parent
+            subproject_slug = relation.alias
+
+        return (main_project, subproject_slug)
+
     def _get_canonical_project(self, project, projects=None):
         """
         Recursively get canonical project for subproject or translations.

readthedocs/rtd_tests/tests/test_resolver.py

@@ -1,14 +1,16 @@
-import django_dynamic_fixture as fixture
 from unittest import mock
+
+import django_dynamic_fixture as fixture
+import pytest
 from django.test import TestCase, override_settings
 
+from readthedocs.builds.constants import EXTERNAL
 from readthedocs.core.resolver import (
     Resolver,
     resolve,
     resolve_domain,
     resolve_path,
 )
-from readthedocs.builds.constants import EXTERNAL
 from readthedocs.projects.constants import PRIVATE
 from readthedocs.projects.models import Domain, Project, ProjectRelationship
 from readthedocs.rtd_tests.utils import create_user
@@ -257,32 +259,6 @@ def test_resolver_force_language_version(self):
             )
             self.assertEqual(url, '/cz/foo/index.html')
 
-    def test_resolver_no_force_translation(self):
-        with override_settings(USE_SUBDOMAIN=False):
-            url = resolve_path(
-                project=self.translation, filename='index.html', language='cz',
-            )
-            self.assertEqual(url, '/docs/pip/ja/latest/index.html')
-        with override_settings(USE_SUBDOMAIN=True):
-            url = resolve_path(
-                project=self.translation, filename='index.html', language='cz',
-            )
-            self.assertEqual(url, '/ja/latest/index.html')
-
-    def test_resolver_no_force_translation_with_version(self):
-        with override_settings(USE_SUBDOMAIN=False):
-            url = resolve_path(
-                project=self.translation, filename='index.html', language='cz',
-                version_slug='foo',
-            )
-            self.assertEqual(url, '/docs/pip/ja/foo/index.html')
-        with override_settings(USE_SUBDOMAIN=True):
-            url = resolve_path(
-                project=self.translation, filename='index.html', language='cz',
-                version_slug='foo',
-            )
-            self.assertEqual(url, '/ja/foo/index.html')
-
 
 class ResolverCanonicalProject(TestCase):
 
@@ -547,6 +523,63 @@ def test_resolver_translation(self):
             url = resolve(project=self.translation)
             self.assertEqual(url, 'http://pip.readthedocs.org/ja/latest/')
 
+    @override_settings(PRODUCTION_DOMAIN='readthedocs.org')
+    def test_resolver_nested_translation_of_a_subproject(self):
+        """The project is a translation, and the main translation is a subproject of a project."""
+        translation = fixture.get(
+            Project,
+            slug='api-es',
+            language='es',
+            users=[self.owner],
+            main_language_project=self.subproject,
+        )
+
+        with override_settings(USE_SUBDOMAIN=False):
+            url = resolve(project=translation)
+            self.assertEqual(
+                url, 'http://readthedocs.org/docs/pip/projects/sub/es/latest/',
+            )
+        with override_settings(USE_SUBDOMAIN=True):
+            url = resolve(project=translation)
+            self.assertEqual(
+                url, 'http://pip.readthedocs.org/projects/sub/es/latest/',
+            )
+
+    @pytest.mark.xfail(reason='We do not support this for now', strict=True)
+    @override_settings(PRODUCTION_DOMAIN='readthedocs.org')
+    def test_resolver_nested_subproject_of_a_translation(self):
+        """The project is a subproject, and the superproject is a translation of a project."""
+        project = fixture.get(
+            Project,
+            slug='all-docs',
+            language='en',
+            users=[self.owner],
+            main_language_project=None,
+        )
+        translation = fixture.get(
+            Project,
+            slug='docs-es',
+            language='es',
+            users=[self.owner],
+            main_language_project=project,
+        )
+
+        subproject = fixture.get(
+            Project,
+            slug='api-es',
+            language='es',
+            users=[self.owner],
+            main_language_project=None,
+        )
+        translation.add_subproject(subproject)
+
+        with override_settings(USE_SUBDOMAIN=False):
+            url = resolve(project=subproject)
+            self.assertEqual(url, 'http://readthedocs.org/docs/docs-es/projects/api-es/es/latest/')
+        with override_settings(USE_SUBDOMAIN=True):
+            url = resolve(project=subproject)
+            self.assertEqual(url, 'http://docs-es.readthedocs.org/projects/api-es/es/latest/')
+
     @override_settings(PRODUCTION_DOMAIN='readthedocs.org')
     def test_resolver_single_version(self):
         self.pip.single_version = True