Skip to content

Remove private argument from resolver #6864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
GITLAB_URL,
MEDIA_TYPES,
PRIVACY_CHOICES,
PRIVATE,
)
from readthedocs.projects.models import APIProject, Project
from readthedocs.projects.version_handling import determine_stable_version
Expand Down Expand Up @@ -307,11 +306,9 @@ def get_absolute_url(self):
'version_slug': self.slug,
},
)
private = self.privacy_level == PRIVATE
external = self.type == EXTERNAL
return self.project.get_docs_url(
version_slug=self.slug,
private=private,
external=external,
)

Expand Down Expand Up @@ -364,12 +361,10 @@ def supports_wipe(self):
return not self.type == EXTERNAL

def get_subdomain_url(self):
private = self.privacy_level == PRIVATE
external = self.type == EXTERNAL
return self.project.get_docs_url(
version_slug=self.slug,
lang_slug=self.project.language,
private=private,
external=external,
)

Expand Down
41 changes: 15 additions & 26 deletions readthedocs/core/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from django.conf import settings

from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.projects.constants import PRIVATE
from readthedocs.builds.constants import EXTERNAL

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -59,7 +58,6 @@ def base_resolve_path(
filename,
version_slug=None,
language=None,
private=False,
single_version=None,
subproject_slug=None,
subdomain=None,
Expand All @@ -68,7 +66,6 @@ def base_resolve_path(
"""Resolve a with nothing smart, just filling in the blanks."""
# Only support `/docs/project' URLs outside our normal environment. Normally
# the path should always have a subdomain or CNAME domain
# pylint: disable=unused-argument
if subdomain or cname or (self._use_subdomain()):
url = '/'
else:
Expand Down Expand Up @@ -100,16 +97,12 @@ def resolve_path(
single_version=None,
subdomain=None,
cname=None,
private=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

if private is None:
private, _ = self._get_private_and_external(project, version_slug)

filename = self._fix_filename(project, filename)

current_project = project
Expand Down Expand Up @@ -144,12 +137,10 @@ def resolve_path(
single_version=single_version,
subproject_slug=subproject_slug,
cname=cname,
private=private,
subdomain=subdomain,
)

def resolve_domain(self, project, private=None):
# pylint: disable=unused-argument
def resolve_domain(self, project):
canonical_project = self._get_canonical_project(project)
domain = canonical_project.get_canonical_custom_domain()
if domain:
Expand All @@ -162,14 +153,14 @@ def resolve_domain(self, project, private=None):

def resolve(
self, project, require_https=False, filename='', query_params='',
private=None, external=None, **kwargs
external=None, **kwargs
):
version_slug = kwargs.get('version_slug')

if private is None or external is None:
if version_slug is None:
version_slug = project.get_default_version()
private, external = self._get_private_and_external(project, version_slug)
if version_slug is None:
version_slug = project.get_default_version()
if external is None:
external = self._is_external(project, version_slug)

canonical_project = self._get_canonical_project(project)
custom_domain = canonical_project.get_canonical_custom_domain()
Expand Down Expand Up @@ -200,7 +191,7 @@ def resolve(
protocol = 'https' if use_https_protocol else 'http'

path = self.resolve_path(
project, filename=filename, private=private, **kwargs
project, filename=filename, **kwargs
)
return urlunparse((protocol, domain, path, '', query_params, ''))

Expand Down Expand Up @@ -245,16 +236,14 @@ def _get_project_subdomain(self, project):
subdomain_slug = project.slug.replace('_', '-')
return '{}.{}'.format(subdomain_slug, settings.PUBLIC_DOMAIN)

def _get_private_and_external(self, project, version_slug):
from readthedocs.builds.models import Version
try:
version = project.versions.get(slug=version_slug)
private = version.privacy_level == PRIVATE
external = version.type == EXTERNAL
except Version.DoesNotExist:
private = settings.DEFAULT_PRIVACY_LEVEL == PRIVATE
external = False
return private, external
def _is_external(self, project, version_slug):
type_ = (
project.versions
.values_list('type', flat=True)
.filter(slug=version_slug)
.first()
)
return type_ == EXTERNAL

def _fix_filename(self, project, filename):
"""
Expand Down
1 change: 0 additions & 1 deletion readthedocs/core/views/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,6 @@ def changefreqs_generator():
href = project.get_docs_url(
version_slug=version.slug,
lang_slug=translation.language,
private=False,
)
element['languages'].append({
'hreflang': hreflang_formatter(translation.language),
Expand Down
3 changes: 1 addition & 2 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ def delete(self, *args, **kwargs): # pylint: disable=arguments-differ
def get_absolute_url(self):
return reverse('projects_detail', args=[self.slug])

def get_docs_url(self, version_slug=None, lang_slug=None, private=None, external=False):
def get_docs_url(self, version_slug=None, lang_slug=None, external=False):
"""
Return a URL for the docs.

Expand All @@ -496,7 +496,6 @@ def get_docs_url(self, version_slug=None, lang_slug=None, private=None, external
project=self,
version_slug=version_slug,
language=lang_slug,
private=private,
external=external,
)

Expand Down
6 changes: 0 additions & 6 deletions readthedocs/proxito/tests/test_full.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,6 @@ def test_sitemap_xml(self):
self.project.get_docs_url(
version_slug=version.slug,
lang_slug=self.project.language,
private=False,
),
)

Expand All @@ -451,7 +450,6 @@ def test_sitemap_xml(self):
self.project.get_docs_url(
version_slug=private_version.slug,
lang_slug=self.project.language,
private=True,
),
)
# The `translation` project doesn't have a version named `not-translated-version`
Expand All @@ -463,7 +461,6 @@ def test_sitemap_xml(self):
self.project.get_docs_url(
version_slug=not_translated_public_version.slug,
lang_slug=translation.language,
private=False,
),
)
# hreflang should use hyphen instead of underscore
Expand All @@ -476,7 +473,6 @@ def test_sitemap_xml(self):
self.project.get_docs_url(
version_slug=external_version.slug,
lang_slug=self.project.language,
private=True,
),
)

Expand All @@ -486,7 +482,6 @@ def test_sitemap_xml(self):
self.project.get_docs_url(
version_slug=stable_version.slug,
lang_slug=self.project.language,
private=False,
),)
self.assertEqual(response.context['versions'][0]['priority'], 1)
self.assertEqual(response.context['versions'][0]['changefreq'], 'weekly')
Expand All @@ -497,7 +492,6 @@ def test_sitemap_xml(self):
self.project.get_docs_url(
version_slug='latest',
lang_slug=self.project.language,
private=False,
),)
self.assertEqual(response.context['versions'][1]['priority'], 0.9)
self.assertEqual(response.context['versions'][1]['changefreq'], 'daily')
Expand Down
1 change: 0 additions & 1 deletion readthedocs/proxito/views/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,6 @@ def changefreqs_generator():
href = project.get_docs_url(
version_slug=version.slug,
lang_slug=translation.language,
private=False,
)
element['languages'].append({
'hreflang': hreflang_formatter(translation.language),
Expand Down
6 changes: 0 additions & 6 deletions readthedocs/rtd_tests/tests/test_doc_serving.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ def test_sitemap_xml(self):
self.public.get_docs_url(
version_slug=version.slug,
lang_slug=self.public.language,
private=False,
),
)

Expand All @@ -294,7 +293,6 @@ def test_sitemap_xml(self):
self.public.get_docs_url(
version_slug=private_version.slug,
lang_slug=self.public.language,
private=True,
),
)
# The `translation` project doesn't have a version named `not-translated-version`
Expand All @@ -306,7 +304,6 @@ def test_sitemap_xml(self):
self.public.get_docs_url(
version_slug=not_translated_public_version.slug,
lang_slug=translation.language,
private=False,
),
)
# hreflang should use hyphen instead of underscore
Expand All @@ -319,7 +316,6 @@ def test_sitemap_xml(self):
self.public.get_docs_url(
version_slug=external_version.slug,
lang_slug=self.public.language,
private=True,
),
)

Expand All @@ -329,7 +325,6 @@ def test_sitemap_xml(self):
self.public.get_docs_url(
version_slug=stable_version.slug,
lang_slug=self.public.language,
private=False,
),)
self.assertEqual(response.context['versions'][0]['priority'], 1)
self.assertEqual(response.context['versions'][0]['changefreq'], 'weekly')
Expand All @@ -340,7 +335,6 @@ def test_sitemap_xml(self):
self.public.get_docs_url(
version_slug='latest',
lang_slug=self.public.language,
private=False,
),)
self.assertEqual(response.context['versions'][1]['priority'], 0.9)
self.assertEqual(response.context['versions'][1]['changefreq'], 'daily')
Expand Down
44 changes: 20 additions & 24 deletions readthedocs/rtd_tests/tests/test_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,13 +463,6 @@ def test_domain_public(self):
with override_settings(USE_SUBDOMAIN=True):
url = resolve_domain(project=self.translation)
self.assertEqual(url, 'pip.public.readthedocs.org')
# Private overrides domain
with override_settings(USE_SUBDOMAIN=False):
url = resolve_domain(project=self.translation, private=True)
self.assertEqual(url, 'readthedocs.org')
with override_settings(USE_SUBDOMAIN=True):
url = resolve_domain(project=self.translation, private=True)
self.assertEqual(url, 'pip.public.readthedocs.org')

@override_settings(
PRODUCTION_DOMAIN='readthedocs.org',
Expand Down Expand Up @@ -584,25 +577,28 @@ def test_resolver_subproject_alias(self):

@override_settings(PRODUCTION_DOMAIN='readthedocs.org')
def test_resolver_private_project(self):
self.pip.privacy_level = PRIVATE
self.pip.save()
with override_settings(USE_SUBDOMAIN=False):
url = resolve(project=self.pip, private=True)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
with override_settings(USE_SUBDOMAIN=True):
url = resolve(project=self.pip, private=True)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')

@override_settings(PRODUCTION_DOMAIN='readthedocs.org')
def test_resolver_private_project_override(self):
self.pip.privacy_level = PRIVATE
self.pip.save()
with override_settings(USE_SUBDOMAIN=False):
url = resolve(project=self.pip)
self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
with override_settings(USE_SUBDOMAIN=True):
url = resolve(project=self.pip)
self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')
url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')

@override_settings(PRODUCTION_DOMAIN='readthedocs.org')
Expand All @@ -613,12 +609,12 @@ def test_resolver_private_version_override(self):
with override_settings(USE_SUBDOMAIN=False):
url = resolve(project=self.pip)
self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
with override_settings(USE_SUBDOMAIN=True):
url = resolve(project=self.pip)
self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')
url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://pip.readthedocs.org/en/latest/')

@override_settings(
Expand All @@ -627,16 +623,16 @@ def test_resolver_private_version_override(self):
)
def test_resolver_public_domain_overrides(self):
with override_settings(USE_SUBDOMAIN=False):
url = resolve(project=self.pip, private=True)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://readthedocs.org/docs/pip/en/latest/')
with override_settings(USE_SUBDOMAIN=True):
url = resolve(project=self.pip, private=True)
url = resolve(project=self.pip)
self.assertEqual(
url, 'http://pip.public.readthedocs.org/en/latest/',
)
url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(
url, 'http://pip.public.readthedocs.org/en/latest/',
)
Expand All @@ -649,14 +645,14 @@ def test_resolver_public_domain_overrides(self):
canonical=True,
)
with override_settings(USE_SUBDOMAIN=True):
url = resolve(project=self.pip, private=True)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://docs.foobar.com/en/latest/')
url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://docs.foobar.com/en/latest/')
with override_settings(USE_SUBDOMAIN=False):
url = resolve(project=self.pip, private=True)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://docs.foobar.com/en/latest/')
url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://docs.foobar.com/en/latest/')

@override_settings(
Expand All @@ -666,14 +662,14 @@ def test_resolver_public_domain_overrides(self):
)
def test_resolver_domain_https(self):
with override_settings(PUBLIC_DOMAIN_USES_HTTPS=True):
url = resolve(project=self.pip, private=True)
url = resolve(project=self.pip)
self.assertEqual(url, 'https://pip.readthedocs.io/en/latest/')

url = resolve(project=self.pip, private=False)
url = resolve(project=self.pip)
self.assertEqual(url, 'https://pip.readthedocs.io/en/latest/')

with override_settings(PUBLIC_DOMAIN_USES_HTTPS=False):
url = resolve(project=self.pip, private=True)
url = resolve(project=self.pip)
self.assertEqual(url, 'http://pip.readthedocs.io/en/latest/')


Expand Down