From fc3b4df3c8a1860e73a6bb3885457e9ea594b6ed Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 14 Aug 2023 17:32:34 -0500 Subject: [PATCH 1/6] Proxito: allow to generate proxied API URLs with a prefix - Ref https://github.com/readthedocs/readthedocs.org/issues/10181 - Ref https://github.com/readthedocs/readthedocs.org/issues/10408 - Ref https://github.com/readthedocs/meta/issues/124 --- readthedocs/api/v2/serializers.py | 1 + readthedocs/projects/models.py | 27 ++++++++++++++++++++------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py index 521a2e7acf4..4fc01664ef0 100644 --- a/readthedocs/api/v2/serializers.py +++ b/readthedocs/api/v2/serializers.py @@ -31,6 +31,7 @@ class Meta: 'users', 'canonical_url', 'urlconf', + 'custom_prefix', ) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index ec4a85f248c..cce987b7a65 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -693,11 +693,9 @@ def proxied_api_host(self): This needs to start with a slash at the root of the domain, and end without a slash """ - if self.urlconf: - # Add our proxied api host at the first place we have a $variable - # This supports both subpaths & normal root hosting - path_prefix = self.custom_path_prefix - return unsafe_join_url_path(path_prefix, "/_") + custom_prefix = self.proxied_api_prefix + if custom_prefix: + return unsafe_join_url_path(custom_prefix, "/_") return '/_' @property @@ -715,12 +713,20 @@ def proxied_static_path(self): return f"{self.proxied_api_host}/static/" @property - def custom_path_prefix(self): + def proxied_api_prefix(self): """ - Get the path prefix from the custom urlconf. + Get the path prefix for proxied API paths (``/_/``). Returns `None` if the project doesn't have a custom urlconf. """ + # When using a custom prefix, we can only handle serving + # docs pages under the prefix, not special paths like `/_/`. + # Projects using the old implementation, need to proxy `/_/` + # paths as is, this is, without the suffix, while those projects + # migrate to the new implementation, we will prefix special paths, + # they are manually un-prefixed in nginx. + if self.custom_prefix and self.has_feature(Feature.USE_PROXIED_APIS_WITH_PREFIX): + return self.custom_prefix if self.urlconf: # Return the value before the first defined variable, # as that is a prefix and not part of our normal doc patterns. @@ -1930,6 +1936,7 @@ def add_features(sender, **kwargs): DISABLE_PAGEVIEWS = "disable_pageviews" RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header" USE_UNRESOLVER_WITH_PROXITO = "use_unresolver_with_proxito" + USE_PROXIED_APIS_WITH_PREFIX = "use_proxied_apis_with_prefix" ALLOW_VERSION_WARNING_BANNER = "allow_version_warning_banner" # Versions sync related features @@ -2020,6 +2027,12 @@ def add_features(sender, **kwargs): "Proxito: Use new unresolver implementation for serving documentation files." ), ), + ( + USE_PROXIED_APIS_WITH_PREFIX, + _( + "Proxito: Use proxied APIs (/_/*) with the custom prefix if the project has one (Project.custom_prefix)." + ), + ), ( ALLOW_VERSION_WARNING_BANNER, _("Dashboard: Allow project to use the version warning banner."), From 97ff71ea6f63ae4f8f5cfa89eefc32d0e7adf73f Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 14 Aug 2023 17:45:46 -0500 Subject: [PATCH 2/6] Test --- readthedocs/projects/tests/test_models.py | 24 ++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/readthedocs/projects/tests/test_models.py b/readthedocs/projects/tests/test_models.py index 4e00121db45..6f6512a961f 100644 --- a/readthedocs/projects/tests/test_models.py +++ b/readthedocs/projects/tests/test_models.py @@ -3,7 +3,7 @@ from django.test import TestCase from django_dynamic_fixture import get -from readthedocs.projects.models import Project +from readthedocs.projects.models import Feature, Project class TestURLPatternsUtils(TestCase): @@ -115,3 +115,25 @@ def test_overlapping_prefixes(self): with pytest.raises(ValidationError) as excinfo: self.project.clean() self.assertEqual(excinfo.value.code, "ambiguous_path") + + def test_proxied_api_prefix(self): + self.assertEqual(self.project.custom_prefix, None) + self.assertEqual(self.project.proxied_api_url, "_/") + self.assertEqual(self.project.proxied_api_host, "/_") + self.assertEqual(self.project.proxied_api_prefix, None) + + self.project.custom_prefix = "/prefix/" + self.project.save() + + self.assertEqual(self.project.proxied_api_url, "_/") + self.assertEqual(self.project.proxied_api_host, "/_") + self.assertEqual(self.project.proxied_api_prefix, None) + + get( + Feature, + projects=[self.project], + feature_id=Feature.USE_PROXIED_APIS_WITH_PREFIX, + ) + self.assertEqual(self.project.proxied_api_url, "prefix/_/") + self.assertEqual(self.project.proxied_api_host, "/prefix/_") + self.assertEqual(self.project.proxied_api_prefix, "/prefix/") From d48eaedbf89274108e79d49c709eaeabcf93b93e Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 14 Aug 2023 17:50:32 -0500 Subject: [PATCH 3/6] Format --- readthedocs/api/v2/serializers.py | 30 +++++++++++++++--------------- readthedocs/projects/models.py | 4 +++- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py index 4fc01664ef0..59d09fc7530 100644 --- a/readthedocs/api/v2/serializers.py +++ b/readthedocs/api/v2/serializers.py @@ -17,21 +17,21 @@ class ProjectSerializer(serializers.ModelSerializer): class Meta: model = Project fields = ( - 'id', - 'name', - 'slug', - 'description', - 'language', - 'programming_language', - 'repo', - 'repo_type', - 'default_version', - 'default_branch', - 'documentation_type', - 'users', - 'canonical_url', - 'urlconf', - 'custom_prefix', + "id", + "name", + "slug", + "description", + "language", + "programming_language", + "repo", + "repo_type", + "default_version", + "default_branch", + "documentation_type", + "users", + "canonical_url", + "urlconf", + "custom_prefix", ) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index cce987b7a65..dcdf3dda5bb 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -725,7 +725,9 @@ def proxied_api_prefix(self): # paths as is, this is, without the suffix, while those projects # migrate to the new implementation, we will prefix special paths, # they are manually un-prefixed in nginx. - if self.custom_prefix and self.has_feature(Feature.USE_PROXIED_APIS_WITH_PREFIX): + if self.custom_prefix and self.has_feature( + Feature.USE_PROXIED_APIS_WITH_PREFIX + ): return self.custom_prefix if self.urlconf: # Return the value before the first defined variable, From 87f2c74b409fca6dfebff34a362e8c5e4660364a Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 14 Aug 2023 17:53:37 -0500 Subject: [PATCH 4/6] Suffix -> prefix --- readthedocs/projects/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index dcdf3dda5bb..283f03dd233 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -722,7 +722,7 @@ def proxied_api_prefix(self): # When using a custom prefix, we can only handle serving # docs pages under the prefix, not special paths like `/_/`. # Projects using the old implementation, need to proxy `/_/` - # paths as is, this is, without the suffix, while those projects + # paths as is, this is, without the prefix, while those projects # migrate to the new implementation, we will prefix special paths, # they are manually un-prefixed in nginx. if self.custom_prefix and self.has_feature( From 7abad73e3f5b48b460864b5d91253ea8015a9f6f Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 14 Aug 2023 17:54:53 -0500 Subject: [PATCH 5/6] Update docstring --- readthedocs/projects/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 283f03dd233..f5fda7a6d24 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -723,8 +723,8 @@ def proxied_api_prefix(self): # docs pages under the prefix, not special paths like `/_/`. # Projects using the old implementation, need to proxy `/_/` # paths as is, this is, without the prefix, while those projects - # migrate to the new implementation, we will prefix special paths, - # they are manually un-prefixed in nginx. + # migrate to the new implementation, we will prefix special paths + # when generating links, these paths will be manually un-prefixed in nginx. if self.custom_prefix and self.has_feature( Feature.USE_PROXIED_APIS_WITH_PREFIX ): From d5266c476a9c76762d50e63d7a1d1f2e557a01a2 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 14 Aug 2023 18:43:46 -0500 Subject: [PATCH 6/6] Fix test --- readthedocs/rtd_tests/tests/test_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 5651970f569..61ea38b2366 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -3176,6 +3176,7 @@ def test_get_version_by_id(self): "use_system_packages": False, "users": [1], "urlconf": None, + "custom_prefix": None, }, "privacy_level": "public", "downloads": {},