Skip to content

Commit

Permalink
Force resolver to use PUBLIC_DOMAIN over HTTPS if not Domain.https (#…
Browse files Browse the repository at this point in the history 
…4579)

* Force resolver to use PUBLIC_DOMAIN over HTTPS if not Domain.https

Argument ``require_https_domain`` added to force the resolver to use
https URLs with PUBLIC_DOMAIN when the Domain is not https.

* Always use HTTPS protocol when require_https_domain is True

* Rename the flag to just `require_https`

* Use a method to decide if we use the custom domain or not

* Remove invalid tests

* Refactor Symlink test cases to use override_settings properly

Doing `settings.SITE_ROOT = '/path'` may cause problems. For this
cases, `@override_settings` has to be used.

This commit refactors the code around Symlink tests to use the
decorator properly.

* Use helper method to decide whether or not use the custom domain
  • Loading branch information
@humitos @agjohnson
humitos authored and agjohnson committed Sep 5, 2018
1 parent 45b6ead commit ef6a059
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 51 deletions.
38 changes: 26 additions & 12 deletions readthedocs/core/resolver.py
View file
Expand Up @@ -136,7 +136,7 @@ def resolve_domain(self, project, private=None):
return self._get_project_subdomain(canonical_project)
return getattr(settings, 'PRODUCTION_DOMAIN')

def resolve(self, project, protocol='http', filename='', private=None,
def resolve(self, project, require_https=False, filename='', private=None,
**kwargs):
if private is None:
version_slug = kwargs.get('version_slug')
Expand All @@ -146,24 +146,27 @@ def resolve(self, project, protocol='http', filename='', private=None,

canonical_project = self._get_canonical_project(project)
custom_domain = self._get_project_custom_domain(canonical_project)
use_custom_domain = self._use_custom_domain(custom_domain)

# This duplication from resolve_domain is for performance purposes
# in order to check whether a custom domain should be HTTPS
if custom_domain:
if use_custom_domain:
domain = custom_domain.domain
elif self._use_subdomain():
domain = self._get_project_subdomain(canonical_project)
else:
domain = getattr(settings, 'PRODUCTION_DOMAIN')

if custom_domain:
protocol = 'https' if custom_domain.https else 'http'
else:
# Use HTTPS if settings specify
public_domain = getattr(settings, 'PUBLIC_DOMAIN', None)
use_https = getattr(settings, 'PUBLIC_DOMAIN_USES_HTTPS', False)
if use_https and public_domain and public_domain in domain:
protocol = 'https'
public_domain = getattr(settings, 'PUBLIC_DOMAIN', None)
use_https = getattr(settings, 'PUBLIC_DOMAIN_USES_HTTPS', False)

use_https_protocol = any([
# Rely on the ``Domain.https`` field
use_custom_domain and custom_domain.https,
# or force it if specified
require_https,
# or fallback to settings
use_https and public_domain and public_domain in domain,
])
protocol = 'https' if use_https_protocol else 'http'

return '{protocol}://{domain}{path}'.format(
protocol=protocol,
Expand Down Expand Up @@ -249,6 +252,17 @@ def _fix_filename(self, project, filename):
path = ""
return path

def _use_custom_domain(self, custom_domain):
"""
Make decision about whether to use a custom domain to serve docs.
Always use the custom domain if it exists.
:param custom_domain: Domain instance or ``None``
:type custom_domain: readthedocs.projects.models.Domain
"""
return True if custom_domain is not None else False

def _use_subdomain(self):
"""Make decision about whether to use a subdomain to serve docs."""
use_subdomain = getattr(settings, 'USE_SUBDOMAIN', False)
Expand Down
69 changes: 30 additions & 39 deletions readthedocs/rtd_tests/tests/test_project_symlinks.py
View file
Expand Up @@ -51,23 +51,26 @@ def get_filesystem(path, top_level_path=None):
return fs


class TempSiterootCase(object):
TEMP_SITE_ROOT = tempfile.mkdtemp(suffix='siteroot')
TEMP_DOCROOT = os.path.join(TEMP_SITE_ROOT, 'user_builds')

"""Override SITE_ROOT and patch necessary pieces to inspect symlink structure

@override_settings(
SITE_ROOT=TEMP_SITE_ROOT,
DOCROOT=TEMP_DOCROOT,
)
class TempSiteRootTestCase(TestCase):

"""
Override SITE_ROOT and patch necessary pieces to inspect symlink structure.
This uses some patches and overidden settings to build out symlinking in a
temporary path. Each test is therefore isolated, and cleanup will remove
these paths after the test case wraps up.
And subclasses that implement :py:class:`TestCase` should also make use of
:py:func:`override_settings`.
"""

def setUp(self):
self.maxDiff = None
self.site_root = os.path.realpath(tempfile.mkdtemp(suffix='siteroot'))
settings.SITE_ROOT = self.site_root
settings.DOCROOT = os.path.join(settings.SITE_ROOT, 'user_builds')
self.mocks = {
'PublicSymlinkBase.CNAME_ROOT': mock.patch(
'readthedocs.core.symlink.PublicSymlinkBase.CNAME_ROOT',
Expand Down Expand Up @@ -115,16 +118,16 @@ def setUp(self):
)

def tearDown(self):
shutil.rmtree(self.site_root)
shutil.rmtree(settings.SITE_ROOT)

def assertFilesystem(self, filesystem):
"""
Creates a nested dictionary that represents the folder structure of rootdir
"""
self.assertEqual(filesystem, get_filesystem(self.site_root))
self.assertEqual(filesystem, get_filesystem(settings.SITE_ROOT))


class BaseSymlinkCnames(TempSiterootCase):
class BaseSymlinkCnames(object):

def setUp(self):
super(BaseSymlinkCnames, self).setUp()
Expand Down Expand Up @@ -290,19 +293,17 @@ def test_symlink_cname_dont_link_missing_domains(self):
self.assertFilesystem(filesystem)


@override_settings()
class TestPublicSymlinkCnames(BaseSymlinkCnames, TestCase):
class TestPublicSymlinkCnames(BaseSymlinkCnames, TempSiteRootTestCase):
privacy = 'public'
symlink_class = PublicSymlink


@override_settings()
class TestPrivateSymlinkCnames(BaseSymlinkCnames, TestCase):
class TestPrivateSymlinkCnames(BaseSymlinkCnames, TempSiteRootTestCase):
privacy = 'private'
symlink_class = PrivateSymlink


class BaseSubprojects(TempSiterootCase):
class BaseSubprojects(object):

def setUp(self):
super(BaseSubprojects, self).setUp()
Expand Down Expand Up @@ -503,19 +504,17 @@ def test_remove_subprojects(self):
self.assertFilesystem(filesystem)


@override_settings()
class TestPublicSubprojects(BaseSubprojects, TestCase):
class TestPublicSubprojects(BaseSubprojects, TempSiteRootTestCase):
privacy = 'public'
symlink_class = PublicSymlink


@override_settings()
class TestPrivateSubprojects(BaseSubprojects, TestCase):
class TestPrivateSubprojects(BaseSubprojects, TempSiteRootTestCase):
privacy = 'private'
symlink_class = PrivateSymlink


class BaseSymlinkTranslations(TempSiterootCase):
class BaseSymlinkTranslations(object):

def setUp(self):
super(BaseSymlinkTranslations, self).setUp()
Expand Down Expand Up @@ -757,19 +756,17 @@ def test_remove_language(self):
self.assertFilesystem(filesystem)


@override_settings()
class TestPublicSymlinkTranslations(BaseSymlinkTranslations, TestCase):
class TestPublicSymlinkTranslations(BaseSymlinkTranslations, TempSiteRootTestCase):
privacy = 'public'
symlink_class = PublicSymlink


@override_settings()
class TestPrivateSymlinkTranslations(BaseSymlinkTranslations, TestCase):
class TestPrivateSymlinkTranslations(BaseSymlinkTranslations, TempSiteRootTestCase):
privacy = 'private'
symlink_class = PrivateSymlink


class BaseSymlinkSingleVersion(TempSiterootCase):
class BaseSymlinkSingleVersion(object):

def setUp(self):
super(BaseSymlinkSingleVersion, self).setUp()
Expand Down Expand Up @@ -834,19 +831,17 @@ def test_symlink_single_version_missing(self):
self.assertFilesystem(filesystem)


@override_settings()
class TestPublicSymlinkSingleVersion(BaseSymlinkSingleVersion, TestCase):
class TestPublicSymlinkSingleVersion(BaseSymlinkSingleVersion, TempSiteRootTestCase):
privacy = 'public'
symlink_class = PublicSymlink


@override_settings()
class TestPublicSymlinkSingleVersion(BaseSymlinkSingleVersion, TestCase):
class TestPublicSymlinkSingleVersion(BaseSymlinkSingleVersion, TempSiteRootTestCase):
privacy = 'private'
symlink_class = PrivateSymlink


class BaseSymlinkVersions(TempSiterootCase):
class BaseSymlinkVersions(object):

def setUp(self):
super(BaseSymlinkVersions, self).setUp()
Expand Down Expand Up @@ -962,20 +957,17 @@ def test_symlink_other_versions(self):
self.assertFilesystem(filesystem)


@override_settings()
class TestPublicSymlinkVersions(BaseSymlinkVersions, TestCase):
class TestPublicSymlinkVersions(BaseSymlinkVersions, TempSiteRootTestCase):
privacy = 'public'
symlink_class = PublicSymlink


@override_settings()
class TestPrivateSymlinkVersions(BaseSymlinkVersions, TestCase):
class TestPrivateSymlinkVersions(BaseSymlinkVersions, TempSiteRootTestCase):
privacy = 'private'
symlink_class = PrivateSymlink


@override_settings()
class TestPublicSymlinkUnicode(TempSiterootCase, TestCase):
class TestPublicSymlinkUnicode(TempSiteRootTestCase):

def setUp(self):
super(TestPublicSymlinkUnicode, self).setUp()
Expand Down Expand Up @@ -1040,8 +1032,7 @@ def test_symlink_broadcast_calls_on_project_save(self):
)


@override_settings()
class TestPublicPrivateSymlink(TempSiterootCase, TestCase):
class TestPublicPrivateSymlink(TempSiteRootTestCase):

def setUp(self):
super(TestPublicPrivateSymlink, self).setUp()
Expand Down

0 comments on commit ef6a059

Please sign in to comment.