From c6b166992a62697783aa2316665bcc4f5f5d28e0 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 7 Jul 2021 17:18:40 -0500 Subject: [PATCH] URLConf: fix support for subprojects --- readthedocs/core/resolver.py | 17 +++- readthedocs/projects/models.py | 43 ++++++--- readthedocs/proxito/tests/test_middleware.py | 91 ++++++++++++++++++++ 3 files changed, 135 insertions(+), 16 deletions(-) diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py index 00e3bee8eb2..d19d34eb14a 100644 --- a/readthedocs/core/resolver.py +++ b/readthedocs/core/resolver.py @@ -96,10 +96,19 @@ def base_resolve_path( '$filename', '{filename}', ) - url = url.replace( - '$subproject', - '{subproject_slug}', - ) + # Remove the subproject from the path if + # we are resolving the main project. + # /{subproject}/foo/bar -> /foo/bar. + if subproject_slug: + url = url.replace( + '$subproject', + '{subproject_slug}', + ) + else: + url = url.replace( + '$subproject/', + '', + ) if '$' in url: log.warning( 'Unconverted variable in a resolver URLConf: url=%s', url diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 0891e13efce..e82da2863c7 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -606,12 +606,13 @@ def proxied_api_host(self): This needs to start with a slash at the root of the domain, and end without a slash """ + url = '_' if self.urlconf: # Add our proxied api host at the first place we have a $variable # This supports both subpaths & normal root hosting url_prefix = self.urlconf.split('$', 1)[0] - return '/' + url_prefix.strip('/') + '/_' - return '/_' + url = url_prefix.strip('/') + '/_' + return '/' + url.strip('/') @property def proxied_api_url(self): @@ -628,33 +629,43 @@ def regex_urlconf(self): Convert User's URLConf into a proper django URLConf. This replaces the user-facing syntax with the regex syntax. + + :returns: A tuple with the urlconf for the main project and subprojects. """ to_convert = re.escape(self.urlconf) + subproject_urlconf = to_convert + main_urlconf = to_convert.replace('\\$subproject\\/', '') + if self.single_version: + main_urlconf = main_urlconf.replace('\\$version\\/', '') + main_urlconf = main_urlconf.replace('\\$language\\/', '') + return self._to_regex_urlconf(main_urlconf), self._to_regex_urlconf(subproject_urlconf) + + def _to_regex_urlconf(self, urlconf): # We should standardize these names so we can loop over them easier - to_convert = to_convert.replace( + urlconf = urlconf.replace( '\\$version', '(?P{regex})'.format(regex=pattern_opts['version_slug']) ) - to_convert = to_convert.replace( + urlconf = urlconf.replace( '\\$language', '(?P{regex})'.format(regex=pattern_opts['lang_slug']) ) - to_convert = to_convert.replace( + urlconf = urlconf.replace( '\\$filename', '(?P{regex})'.format(regex=pattern_opts['filename_slug']) ) - to_convert = to_convert.replace( + urlconf = urlconf.replace( '\\$subproject', '(?P{regex})'.format(regex=pattern_opts['project_slug']) ) - if '\\$' in to_convert: + if '\\$' in urlconf: log.warning( - 'Unconverted variable in a project URLConf: project=%s to_convert=%s', - self, to_convert + 'Unconverted variable in a project URLConf: project=%s urlconf=%s', + self, urlconf ) - return to_convert + return urlconf @property def proxito_urlconf(self): @@ -664,9 +675,9 @@ def proxito_urlconf(self): It is used for doc serving on projects that have their own ``urlconf``. """ from readthedocs.projects.views.public import ProjectDownloadMedia + from readthedocs.proxito.urls import core_urls from readthedocs.proxito.views.serve import ServeDocs from readthedocs.proxito.views.utils import proxito_404_page_handler - from readthedocs.proxito.urls import core_urls class ProxitoURLConf: @@ -691,12 +702,20 @@ class ProxitoURLConf: name='user_proxied_downloads' ), ] + + main_urlconf, subproject_urlconf = self.regex_urlconf docs_urls = [ re_path( - '^{regex_urlconf}$'.format(regex_urlconf=self.regex_urlconf), + '^{regex_urlconf}$'.format(regex_urlconf=subproject_urlconf), + ServeDocs.as_view(), + name='user_proxied_serve_docs_subprojects' + ), + re_path( + '^{regex_urlconf}$'.format(regex_urlconf=main_urlconf), ServeDocs.as_view(), name='user_proxied_serve_docs' ), + # paths for redirects at the root re_path( '^{proxied_api_url}$'.format( diff --git a/readthedocs/proxito/tests/test_middleware.py b/readthedocs/proxito/tests/test_middleware.py index 84a925ae5ad..e1277d81701 100644 --- a/readthedocs/proxito/tests/test_middleware.py +++ b/readthedocs/proxito/tests/test_middleware.py @@ -385,3 +385,94 @@ def test_middleware_urlconf_subproject(self): resp['X-Accel-Redirect'], '/proxito/media/html/subproject/testing/foodex.html', ) + + # The main project still works. + resp = self.client.get('/subpath/latest/en/foo.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/pip/latest/foo.html', + ) + + resp = self.client.get('/subpath/latest/en/foo/bar/index.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/pip/latest/foo/bar/index.html', + ) + + def test_serve_subprojects_from_root(self): + self.pip.urlconf = '$subproject/$language/$version/$filename' + self.pip.save() + resp = self.client.get('/subproject/en/testing/foodex.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/subproject/testing/foodex.html', + ) + + # The main project still works. + resp = self.client.get('/en/latest/foo.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/pip/latest/foo.html', + ) + + resp = self.client.get('/en/latest/foo/bar/index.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/pip/latest/foo/bar/index.html', + ) + + def test_middleware_urlconf_subproject_main_project_single_version(self): + self.pip.single_version = True + self.pip.save() + resp = self.client.get('/subpath/subproject/testing/en/foodex.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/subproject/testing/foodex.html', + ) + + # The main project still works. + resp = self.client.get('/subpath/foo.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/pip/latest/foo.html', + ) + + resp = self.client.get('/subpath/foo/bar/index.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/pip/latest/foo/bar/index.html', + ) + + def test_serve_subprojects_from_root_main_project_single_version(self): + self.pip.urlconf = '$subproject/$language/$version/$filename' + self.pip.single_version = True + self.pip.save() + resp = self.client.get('/subproject/en/testing/foodex.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/subproject/testing/foodex.html', + ) + + # The main project still works. + resp = self.client.get('/foo.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/pip/latest/foo.html', + ) + + resp = self.client.get('/foo/bar/index.html', HTTP_HOST=self.domain) + self.assertEqual(resp.status_code, 200) + self.assertEqual( + resp['X-Accel-Redirect'], + '/proxito/media/html/pip/latest/foo/bar/index.html', + )