From 0bcb26a2c499594901d49d9af455d4c3894cc765 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 9 May 2023 20:52:53 -0500 Subject: [PATCH 1/5] Proxito: redirect to default version from root language Closes https://github.com/readthedocs/readthedocs.org/issues/2968 --- readthedocs/core/unresolver.py | 14 ++++++ readthedocs/proxito/tests/test_redirects.py | 49 +++++++++++++++++++ readthedocs/proxito/views/serve.py | 20 ++++++++ .../rtd_tests/tests/test_unresolver.py | 27 +++++++++- 4 files changed, 109 insertions(+), 1 deletion(-) diff --git a/readthedocs/core/unresolver.py b/readthedocs/core/unresolver.py index 65c0d387b2e..508d43ceb0b 100644 --- a/readthedocs/core/unresolver.py +++ b/readthedocs/core/unresolver.py @@ -60,6 +60,12 @@ def __init__(self, project, language, version_slug, filename): self.version_slug = version_slug +class TranslationWithoutVersionError(UnresolverError): + def __init__(self, project, language): + self.project = project + self.language = language + + class InvalidPathForVersionedProjectError(UnresolverError): def __init__(self, project, path): self.project = project @@ -272,6 +278,14 @@ def _match_multiversion_project( filename=file, ) + # If only the language part was given, + # we can't resolve the version. + if version_slug is None: + raise TranslationWithoutVersionError( + project=project, + language=language, + ) + if external_version_slug and external_version_slug != version_slug: raise InvalidExternalVersionError( project=project, diff --git a/readthedocs/proxito/tests/test_redirects.py b/readthedocs/proxito/tests/test_redirects.py index dca319a12ae..6bd6d430b13 100644 --- a/readthedocs/proxito/tests/test_redirects.py +++ b/readthedocs/proxito/tests/test_redirects.py @@ -2,6 +2,8 @@ from django.test import override_settings from django_dynamic_fixture import get +from readthedocs.builds.models import Version +from readthedocs.projects.constants import PUBLIC from readthedocs.projects.models import Feature from readthedocs.proxito.constants import RedirectType from readthedocs.subscriptions.constants import TYPE_CNAME @@ -11,6 +13,7 @@ @override_settings( PUBLIC_DOMAIN='dev.readthedocs.io', + RTD_EXTERNAL_VERSION_DOMAIN="dev.readthedocs.build", PUBLIC_DOMAIN_USES_HTTPS=True, RTD_DEFAULT_FEATURES={ TYPE_CNAME: 1, @@ -412,3 +415,49 @@ def setUp(self): default_true=True, future_default_true=True, ) + + def test_redirect_from_root_language_to_default_version(self): + paths = [ + "/en", + "/en/", + ] + for path in paths: + r = self.client.get( + path, + secure=True, + HTTP_HOST="project.dev.readthedocs.io", + ) + self.assertEqual(r.status_code, 302) + self.assertEqual( + r["Location"], + "https://project.dev.readthedocs.io/en/latest/", + ) + self.assertEqual(r.headers["CDN-Cache-Control"], "public") + self.assertEqual(r.headers["Cache-Tag"], "project") + self.assertEqual(r.headers["X-RTD-Redirect"], RedirectType.system.name) + + def test_redirect_from_root_language_to_default_external_version(self): + get( + Version, + slug="10", + project=self.project, + privacy_level=PUBLIC, + ) + paths = [ + "/en", + "/en/", + ] + for path in paths: + r = self.client.get( + path, + secure=True, + HTTP_HOST="project--10.dev.readthedocs.build", + ) + self.assertEqual(r.status_code, 302) + self.assertEqual( + r["Location"], + "https://project--10.dev.readthedocs.build/en/10/", + ) + self.assertEqual(r.headers["CDN-Cache-Control"], "public") + self.assertEqual(r.headers["Cache-Tag"], "project") + self.assertEqual(r.headers["X-RTD-Redirect"], RedirectType.system.name) diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py index c14a5b0b400..511d12ab67c 100644 --- a/readthedocs/proxito/views/serve.py +++ b/readthedocs/proxito/views/serve.py @@ -19,6 +19,7 @@ InvalidExternalVersionError, InvalidPathForVersionedProjectError, TranslationNotFoundError, + TranslationWithoutVersionError, VersionNotFoundError, unresolver, ) @@ -357,6 +358,25 @@ def get_using_unresolver(self, request): # TODO: find a better way to pass this to the middleware. request.path_project_slug = exc.project.slug raise Http404 + except TranslationWithoutVersionError as exc: + project = exc.project + # TODO: find a better way to pass this to the middleware. + request.path_project_slug = project.slug + + if unresolved_domain.is_from_external_domain: + version_slug = unresolved_domain.external_version_slug + else: + version_slug = None + # Redirect to the default version of the current translation. + # This is `/en -> /en/latest/` or + # `/projects/subproject/en/ -> /projects/subproject/en/latest/`. + return self.system_redirect( + request=request, + final_project=project, + version_slug=version_slug, + filename="", + is_external_version=unresolved_domain.is_from_external_domain, + ) except InvalidPathForVersionedProjectError as exc: project = exc.project if unresolved_domain.is_from_external_domain: diff --git a/readthedocs/rtd_tests/tests/test_unresolver.py b/readthedocs/rtd_tests/tests/test_unresolver.py index 4011657f80a..8bc2fb4f221 100644 --- a/readthedocs/rtd_tests/tests/test_unresolver.py +++ b/readthedocs/rtd_tests/tests/test_unresolver.py @@ -12,6 +12,7 @@ InvalidPathForVersionedProjectError, SuspiciousHostnameError, TranslationNotFoundError, + TranslationWithoutVersionError, VersionNotFoundError, unresolve, ) @@ -99,10 +100,34 @@ def test_path_no_version(self): "https://pip.readthedocs.io/en/", ] for url in urls: - with pytest.raises(VersionNotFoundError) as excinfo: + with pytest.raises(TranslationWithoutVersionError) as excinfo: unresolve(url) exc = excinfo.value self.assertEqual(exc.project, self.pip) + self.assertEqual(exc.language, "en") + + urls = [ + "https://pip.readthedocs.io/ja", + "https://pip.readthedocs.io/ja/", + ] + for url in urls: + with pytest.raises(TranslationWithoutVersionError) as excinfo: + unresolve(url) + exc = excinfo.value + self.assertEqual(exc.project, self.translation) + self.assertEqual(exc.language, "ja") + + def test_invalid_language_no_version(self): + urls = [ + "https://pip.readthedocs.io/es", + "https://pip.readthedocs.io/es/", + ] + for url in urls: + with pytest.raises(TranslationNotFoundError) as excinfo: + unresolve(url) + exc = excinfo.value + self.assertEqual(exc.project, self.pip) + self.assertEqual(exc.language, "es") self.assertEqual(exc.version_slug, None) self.assertEqual(exc.filename, "/") From d125fe026b93ac594c41c3a16ff2bb77737c850c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 9 May 2023 21:11:38 -0500 Subject: [PATCH 2/5] Docs --- docs/user/user-defined-redirects.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/user/user-defined-redirects.rst b/docs/user/user-defined-redirects.rst index 77c00d7a4f5..193c674016b 100644 --- a/docs/user/user-defined-redirects.rst +++ b/docs/user/user-defined-redirects.rst @@ -84,6 +84,20 @@ For example:: You can choose which is the :term:`default version` for Read the Docs to display. This usually corresponds to the most recent official release from your project. +Root language redirect at ``//`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A link to the root language of your documentation (`.readthedocs.io/en/`) +will redirect to the :term:`default version` of that translation, +as set in your project settings. + +This works for readthedocs.io (|org_brand|) and :doc:`custom domains `. +For |com_brand| this can be enabled by contacting :doc:`support `. + +For example:: + + https://docs.readthedocs.io/en/ -> https://docs.readthedocs.io/en/stable/ + Shortlink with ``https://.rtfd.io`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From f403c3c89609e2666872f7d153ab1cd4407fe6e7 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 10 May 2023 13:20:39 -0500 Subject: [PATCH 3/5] Handle exception in 404 view --- readthedocs/proxito/views/serve.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py index 511d12ab67c..f1071b005a9 100644 --- a/readthedocs/proxito/views/serve.py +++ b/readthedocs/proxito/views/serve.py @@ -765,7 +765,9 @@ def get_using_unresolver(self, request, path): project = None version = None - filename = None + # If we weren't able to resolve a filename, + # then the path is the filename. + filename = path lang_slug = None version_slug = None # Try to map the current path to a project/version/filename. @@ -798,6 +800,10 @@ def get_using_unresolver(self, request, path): version_slug = exc.version_slug filename = exc.filename contextualized_404_class = ProjectTranslationHttp404 + except TranslationWithoutVersionError as exc: + project = exc.project + lang_slug = exc.language + # TODO: Use a contextualized 404 except InvalidExternalVersionError as exc: project = exc.project # TODO: Use a contextualized 404 From bbcdc0566bf10c136bbc617925a5472b1399a3cd Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 11 May 2023 18:28:50 -0500 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Benjamin Balder Bach --- docs/user/user-defined-redirects.rst | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/user/user-defined-redirects.rst b/docs/user/user-defined-redirects.rst index 193c674016b..d71119436c9 100644 --- a/docs/user/user-defined-redirects.rst +++ b/docs/user/user-defined-redirects.rst @@ -87,12 +87,14 @@ This usually corresponds to the most recent official release from your project. Root language redirect at ``//`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -A link to the root language of your documentation (`.readthedocs.io/en/`) -will redirect to the :term:`default version` of that translation, -as set in your project settings. +A link to the root language of your documentation (``.readthedocs.io/en/``) +will redirect to the :term:`default version` of that translation. + +.. TODO: Remove this once the feature is default on .com + +This redirect is currently only active on |org_brand| (``.readthedocs.io`` and :doc:`custom domains `). -This works for readthedocs.io (|org_brand|) and :doc:`custom domains `. -For |com_brand| this can be enabled by contacting :doc:`support `. +Root language redirects on |com_brand| can be enabled by contacting :doc:`support `. For example:: From 8182b5b898f66ee5849de4d5d8a1194511fb00f2 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 24 May 2023 10:28:43 -0500 Subject: [PATCH 5/5] Updates from review --- docs/user/user-defined-redirects.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/user/user-defined-redirects.rst b/docs/user/user-defined-redirects.rst index d71119436c9..fe9e4dafcb3 100644 --- a/docs/user/user-defined-redirects.rst +++ b/docs/user/user-defined-redirects.rst @@ -88,7 +88,7 @@ Root language redirect at ``//`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ A link to the root language of your documentation (``.readthedocs.io/en/``) -will redirect to the :term:`default version` of that translation. +will redirect to the :term:`default version` of that language. .. TODO: Remove this once the feature is default on .com @@ -96,7 +96,7 @@ This redirect is currently only active on |org_brand| (``.readthedocs.io`` Root language redirects on |com_brand| can be enabled by contacting :doc:`support `. -For example:: +For example, accessing the English language of the project will redirect you to the its version (``stable``):: https://docs.readthedocs.io/en/ -> https://docs.readthedocs.io/en/stable/