Changes and fixes to symlinking and serving (#2611)

* Add symlinking overrides and version manager queryset changes

* More changes to the extended modeling

* Add some cleanup and more docs

* Test filesystem explicitly

* Expand test coverage, explicitly handle project/version privacy

Author: agjohnson

readthedocs/builds/models.py

@@ -12,8 +12,8 @@
 from taggit.managers import TaggableManager
 
 from readthedocs.core.utils import broadcast
-from readthedocs.privacy.loader import (VersionManager, RelatedBuildManager,
-                                        BuildManager)
+from readthedocs.privacy.backend import VersionQuerySet, VersionManager
+from readthedocs.privacy.loader import RelatedBuildManager, BuildManager
 from readthedocs.projects.models import Project
 from readthedocs.projects.constants import (PRIVACY_CHOICES, GITHUB_URL,
                                             GITHUB_REGEXS, BITBUCKET_URL,
@@ -69,7 +69,8 @@ class Version(models.Model):
     )
     tags = TaggableManager(blank=True)
     machine = models.BooleanField(_('Machine Created'), default=False)
-    objects = VersionManager()
+
+    objects = VersionManager.from_queryset(VersionQuerySet)()
 
     class Meta:
         unique_together = [('project', 'slug')]

readthedocs/core/symlink.py

@@ -61,6 +61,7 @@
 from django.conf import settings
 
 from readthedocs.builds.models import Version
+from readthedocs.core.utils.extend import SettingsOverrideObject
 from readthedocs.projects import constants
 from readthedocs.projects.models import Domain
 from readthedocs.projects.utils import run
@@ -293,7 +294,7 @@ def get_default_version(self):
             return None
 
 
-class PublicSymlink(Symlink):
+class PublicSymlinkBase(Symlink):
     CNAME_ROOT = os.path.join(settings.SITE_ROOT, 'public_cname_root')
     WEB_ROOT = os.path.join(settings.SITE_ROOT, 'public_web_root')
     PROJECT_CNAME_ROOT = os.path.join(settings.SITE_ROOT, 'public_cname_project')
@@ -309,7 +310,7 @@ def get_translations(self):
         return self.project.translations.protected()
 
 
-class PrivateSymlink(Symlink):
+class PrivateSymlinkBase(Symlink):
     CNAME_ROOT = os.path.join(settings.SITE_ROOT, 'private_cname_root')
     WEB_ROOT = os.path.join(settings.SITE_ROOT, 'private_web_root')
     PROJECT_CNAME_ROOT = os.path.join(settings.SITE_ROOT, 'private_cname_project')
@@ -323,3 +324,13 @@ def get_subprojects(self):
 
     def get_translations(self):
         return self.project.translations.private()
+
+
+class PublicSymlink(SettingsOverrideObject):
+
+    _default_class = PublicSymlinkBase
+
+
+class PrivateSymlink(SettingsOverrideObject):
+
+    _default_class = PrivateSymlinkBase

readthedocs/core/utils/extend.py

@@ -4,51 +4,72 @@
 
 from django.conf import settings
 from django.utils.module_loading import import_by_path
-from django.utils.functional import LazyObject
 
 
-class SettingsOverrideObject(LazyObject):
+def get_override_class(proxy_class, default_class=None):
+    """Determine which class to use in an override class
+
+    The `proxy_class` is the main class that is used, and `default_class` is the
+    default class that this proxy class will instantiate.  If `default_class` is
+    not defined, this will be inferred from the `proxy_class`, as is defined in
+    :py:cls:`SettingsOverrideObject`.
+    """
+    if default_class is None:
+        default_class = getattr(proxy_class, '_default_class')
+    class_id = '.'.join([
+        inspect.getmodule(proxy_class).__name__,
+        proxy_class.__name__
+    ])
+    class_path = getattr(settings, 'CLASS_OVERRIDES', {}).get(class_id)
+    if class_path is None and proxy_class._override_setting is not None:
+        class_path = getattr(settings, proxy_class._override_setting, None)
+    if class_path is not None:
+        default_class = import_by_path(class_path)
+    return default_class
+
+
+class SettingsOverrideMeta(type):
+
+    """Meta class for passing along classmethod class to the underlying class"""
+
+    def __getattr__(cls, attr):  # noqa: pep8 false positive
+        proxy_class = getattr(cls, '_default_class')
+        return getattr(proxy_class, attr)
+
+
+class SettingsOverrideObject(object):
 
     """Base class for creating class that can be overridden
 
     This is used for extension points in the code, where we want to extend a
-    class without monkey patching it. This abstract class allows for lazy
-    inheritance, creating a class from the specified class or from a setting,
-    but only once the class is called.
+    class without monkey patching it. This class will proxy classmethod calls
+    and instantiation to an underlying class, determined by used of
+    :py:cvar:`_default_class` or an override class from settings.
 
-    Default to an instance of the class defined by :py:cvar:`_default_class`.
+    The default target class is defined by :py:cvar:`_default_class`.
 
-    Next, look for an override setting class path in
-    ``settings.CLASS_OVERRIDES``, which should be a dictionary of class paths.
-    The setting should be a dictionary keyed by the object path name::
+    To override this class, an override setting class path can be added to
+    ``settings.CLASS_OVERRIDES``. This settings should be a dictionary keyed by
+    source class paths, with values to the override classes::
 
         CLASS_OVERRIDES = {
             'readthedocs.core.resolver.Resolver': 'something.resolver.Resolver',
         }
 
     Lastly, if ``settings.CLASS_OVERRIDES`` is missing, or the key is not found,
-    attempt to pull the key :py:cvar:`_override_setting` from ``settings``.
+    attempt to pull the key :py:cvar:`_override_setting` from ``settings``. This
+    matches the pattern we've been using previously.
     """
 
+    __metaclass__ = SettingsOverrideMeta
+
     _default_class = None
     _override_setting = None
 
-    def _setup(self):
+    def __new__(cls, *args, **kwargs):
         """Set up wrapped object
 
-        This is called when attributes are accessed on :py:class:`LazyObject`
-        and the underlying wrapped object does not yet exist.
+        Create an instance of the underlying target class and return instead of
+        this class.
         """
-        cls = self._default_class
-        cls_path = (getattr(settings, 'CLASS_OVERRIDES', {})
-                    .get(self._get_class_id()))
-        if cls_path is None and self._override_setting is not None:
-            cls_path = getattr(settings, self._override_setting, None)
-        if cls_path is not None:
-            cls = import_by_path(cls_path)
-        self._wrapped = cls()
-
-    def _get_class_id(self):
-        # type() here, because LazyObject overrides some attribute access
-        return '.'.join([inspect.getmodule(type(self)).__name__,
-                         type(self).__name__])
+        return get_override_class(cls, cls._default_class)(*args, **kwargs)

readthedocs/privacy/backend.py

@@ -1,4 +1,3 @@
-
 from django.db import models
 
 from guardian.shortcuts import get_objects_for_user
@@ -9,6 +8,8 @@
 from readthedocs.builds.constants import LATEST_VERBOSE_NAME
 from readthedocs.builds.constants import STABLE
 from readthedocs.builds.constants import STABLE_VERBOSE_NAME
+from readthedocs.core.utils.extend import (SettingsOverrideObject,
+                                           get_override_class)
 from readthedocs.projects import constants
 
 
@@ -75,6 +76,50 @@ def api(self, user=None):
 
 class VersionManager(models.Manager):
 
+    """Version manager for manager only queries
+
+    For queries not suitable for the :py:cls:`VersionQuerySet`, such as create
+    queries.
+    """
+
+    @classmethod
+    def from_queryset(cls, queryset_class, class_name=None):
+        # This is overridden because :py:meth:`models.Manager.from_queryset`
+        # uses `inspect` to retrieve the class methods, and the proxy class has
+        # no direct members.
+        queryset_class = get_override_class(
+            VersionQuerySet,
+            VersionQuerySet._default_class
+        )
+        return super(VersionManager, cls).from_queryset(queryset_class, class_name)
+
+    def create_stable(self, **kwargs):
+        defaults = {
+            'slug': STABLE,
+            'verbose_name': STABLE_VERBOSE_NAME,
+            'machine': True,
+            'active': True,
+            'identifier': STABLE,
+            'type': TAG,
+        }
+        defaults.update(kwargs)
+        return self.create(**defaults)
+
+    def create_latest(self, **kwargs):
+        defaults = {
+            'slug': LATEST,
+            'verbose_name': LATEST_VERBOSE_NAME,
+            'machine': True,
+            'active': True,
+            'identifier': LATEST,
+            'type': BRANCH,
+        }
+        defaults.update(kwargs)
+        return self.create(**defaults)
+
+
+class VersionQuerySetBase(models.QuerySet):
+
     """
     Versions take into account their own privacy_level setting.
     """
@@ -83,7 +128,7 @@ class VersionManager(models.Manager):
 
     def _add_user_repos(self, queryset, user):
         if user.has_perm('builds.view_version'):
-            return self.get_queryset().all().distinct()
+            return self.all().distinct()
         if user.is_authenticated():
             user_queryset = get_objects_for_user(user, 'builds.view_version')
             queryset = user_queryset | queryset
@@ -122,29 +167,17 @@ def private(self, user=None, project=None, only_active=True):
     def api(self, user=None):
         return self.public(user, only_active=False)
 
-    def create_stable(self, **kwargs):
-        defaults = {
-            'slug': STABLE,
-            'verbose_name': STABLE_VERBOSE_NAME,
-            'machine': True,
-            'active': True,
-            'identifier': STABLE,
-            'type': TAG,
-        }
-        defaults.update(kwargs)
-        return self.create(**defaults)
+    def for_project(self, project):
+        """Return all versions for a project, including translations"""
+        return self.filter(
+            models.Q(project=project) |
+            models.Q(project__main_language_project=project)
+        )
 
-    def create_latest(self, **kwargs):
-        defaults = {
-            'slug': LATEST,
-            'verbose_name': LATEST_VERBOSE_NAME,
-            'machine': True,
-            'active': True,
-            'identifier': LATEST,
-            'type': BRANCH,
-        }
-        defaults.update(kwargs)
-        return self.create(**defaults)
+
+class VersionQuerySet(SettingsOverrideObject):
+
+    _default_class = VersionQuerySetBase
 
 
 class BuildManager(models.Manager):

readthedocs/privacy/loader.py

@@ -5,9 +5,7 @@
 ProjectManager = import_by_path(
     getattr(settings, 'PROJECT_MANAGER',
             'readthedocs.privacy.backend.ProjectManager'))
-VersionManager = import_by_path(
-    getattr(settings, 'VERSION_MANAGER',
-            'readthedocs.privacy.backend.VersionManager'))
+# VersionQuerySet was replaced by SettingsOverrideObject
 BuildManager = import_by_path(
     getattr(settings, 'BUILD_MANAGER',
             'readthedocs.privacy.backend.BuildManager'))

readthedocs/rtd_tests/tests/test_extend.py

@@ -1,6 +1,7 @@
 from django.test import TestCase, override_settings
 
-from readthedocs.core.utils.extend import SettingsOverrideObject
+from readthedocs.core.utils.extend import (SettingsOverrideObject,
+                                           get_override_class)
 
 
 # Top level to ensure module name is correct
@@ -29,10 +30,12 @@ class Foo(SettingsOverrideObject):
             _override_setting = 'FOO_OVERRIDE_CLASS'
 
         foo = Foo()
-        self.assertEqual(foo._get_class_id(), EXTEND_PATH)
         self.assertEqual(foo.__class__.__name__, 'FooBase')
         self.assertEqual(foo.bar(), 1)
 
+        override_class = get_override_class(Foo, Foo._default_class)
+        self.assertEqual(override_class, FooBase)
+
     @override_settings(FOO_OVERRIDE_CLASS=EXTEND_OVERRIDE_PATH)
     def test_with_basic_override(self):
         """Test class override setting defined"""
@@ -41,10 +44,12 @@ class Foo(SettingsOverrideObject):
             _override_setting = 'FOO_OVERRIDE_CLASS'
 
         foo = Foo()
-        self.assertEqual(foo._get_class_id(), EXTEND_PATH)
         self.assertEqual(foo.__class__.__name__, 'NewFoo')
         self.assertEqual(foo.bar(), 2)
 
+        override_class = get_override_class(Foo, Foo._default_class)
+        self.assertEqual(override_class, NewFoo)
+
     @override_settings(FOO_OVERRIDE_CLASS=None,
                        CLASS_OVERRIDES={
                            EXTEND_PATH: EXTEND_OVERRIDE_PATH,
@@ -56,10 +61,12 @@ class Foo(SettingsOverrideObject):
             _override_setting = 'FOO_OVERRIDE_CLASS'
 
         foo = Foo()
-        self.assertEqual(foo._get_class_id(), EXTEND_PATH)
         self.assertEqual(foo.__class__.__name__, 'NewFoo')
         self.assertEqual(foo.bar(), 2)
 
+        override_class = get_override_class(Foo, Foo._default_class)
+        self.assertEqual(override_class, NewFoo)
+
     @override_settings(FOO_OVERRIDE_CLASS=None,
                        CLASS_OVERRIDES={
                            EXTEND_PATH: EXTEND_OVERRIDE_PATH,
@@ -70,6 +77,8 @@ class Foo(SettingsOverrideObject):
             _default_class = FooBase
 
         foo = Foo()
-        self.assertEqual(foo._get_class_id(), EXTEND_PATH)
         self.assertEqual(foo.__class__.__name__, 'NewFoo')
         self.assertEqual(foo.bar(), 2)
+
+        override_class = get_override_class(Foo, Foo._default_class)
+        self.assertEqual(override_class, NewFoo)