diff --git a/readthedocs/builds/migrations/0004_add-apiversion-proxy-model.py b/readthedocs/builds/migrations/0004_add-apiversion-proxy-model.py new file mode 100644 index 00000000000..b96db28e95a --- /dev/null +++ b/readthedocs/builds/migrations/0004_add-apiversion-proxy-model.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.12 on 2017-10-27 00:17 +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0003_add-cold-storage'), + ] + + operations = [ + migrations.CreateModel( + name='APIVersion', + fields=[ + ], + options={ + 'proxy': True, + }, + bases=('builds.version',), + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index e186db2d2c1..f3927178987 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -26,7 +26,7 @@ from readthedocs.projects.constants import (PRIVACY_CHOICES, GITHUB_URL, GITHUB_REGEXS, BITBUCKET_URL, BITBUCKET_REGEXS, PRIVATE) -from readthedocs.projects.models import Project +from readthedocs.projects.models import Project, APIProject DEFAULT_VERSION_PRIVACY_LEVEL = getattr(settings, 'DEFAULT_VERSION_PRIVACY_LEVEL', 'public') @@ -301,6 +301,40 @@ def get_bitbucket_url(self, docroot, filename, source_suffix='.rst'): ) +class APIVersion(Version): + + """Version proxy model for API data deserialization + + This replaces the pattern where API data was deserialized into a mocked + :py:cls:`Version` object. This pattern was confusing, as it was not explicit + as to what form of object you were working with -- API backed or database + backed. + + This model preserves the Version model methods, allowing for overrides on + model field differences. This model pattern will generally only be used on + builder instances, where we are interacting solely with API data. + """ + + project = None + + class Meta: + proxy = True + + def __init__(self, *args, **kwargs): + self.project = APIProject(**kwargs.pop('project', {})) + # These fields only exist on the API return, not on the model, so we'll + # remove them to avoid throwing exceptions due to unexpected fields + for key in ['resource_uri', 'absolute_url', 'downloads']: + try: + del kwargs[key] + except KeyError: + pass + super(APIVersion, self).__init__(*args, **kwargs) + + def save(self, *args, **kwargs): + return 0 + + @python_2_unicode_compatible class VersionAlias(models.Model): diff --git a/readthedocs/core/management/commands/update_api.py b/readthedocs/core/management/commands/update_api.py index 09dbbf95e2c..27d176f5ebb 100644 --- a/readthedocs/core/management/commands/update_api.py +++ b/readthedocs/core/management/commands/update_api.py @@ -10,8 +10,10 @@ import logging from django.core.management.base import BaseCommand -from readthedocs.projects import tasks + from readthedocs.api.client import api +from readthedocs.projects import tasks +from readthedocs.projects.models import APIProject log = logging.getLogger(__name__) @@ -29,6 +31,6 @@ def handle(self, *args, **options): docker = options.get('docker', False) for slug in options['projects']: project_data = api.project(slug).get() - p = tasks.make_api_project(project_data) + p = APIProject(**project_data) log.info("Building %s", p) tasks.update_docs.run(pk=p.pk, docker=docker) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index f5c55f5f44f..457f22dba7c 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -11,6 +11,7 @@ from readthedocs.doc_builder.config import ConfigWrapper from readthedocs.doc_builder.loader import get_builder_class from readthedocs.projects.constants import LOG_TEMPLATE +from readthedocs.projects.models import Feature log = logging.getLogger(__name__) @@ -128,8 +129,16 @@ def install_core_requirements(self): if self.project.documentation_type == 'mkdocs': requirements.append('mkdocs==0.15.0') else: - requirements.extend(['sphinx==1.5.3', 'sphinx-rtd-theme<0.3', - 'readthedocs-sphinx-ext<0.6']) + if self.project.has_feature(Feature.USE_SPHINX_LATEST): + # We will assume semver here and only automate up to the next + # backward incompatible release: 2.x + requirements.append('sphinx<2') + else: + requirements.append('sphinx==1.5.6') + requirements.extend([ + 'sphinx-rtd-theme<0.3', + 'readthedocs-sphinx-ext<0.6' + ]) cmd = [ 'python', diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index c881189e63e..faa3690e04d 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -13,9 +13,10 @@ from readthedocs.redirects.models import Redirect from readthedocs.notifications.views import SendNotificationView -from .notifications import ResourceUsageNotification -from .models import (Project, ImportedFile, +from .forms import FeatureForm +from .models import (Project, ImportedFile, Feature, ProjectRelationship, EmailHook, WebHook, Domain) +from .notifications import ResourceUsageNotification from .tasks import remove_dir @@ -180,8 +181,21 @@ class DomainAdmin(admin.ModelAdmin): model = Domain +class FeatureAdmin(admin.ModelAdmin): + model = Feature + form = FeatureForm + list_display = ('feature_id', 'project_count', 'default_true') + search_fields = ('feature_id',) + filter_horizontal = ('projects',) + readonly_fields = ('add_date',) + + def project_count(self, feature): + return feature.projects.count() + + admin.site.register(Project, ProjectAdmin) admin.site.register(ImportedFile, ImportedFileAdmin) admin.site.register(Domain, DomainAdmin) +admin.site.register(Feature, FeatureAdmin) admin.site.register(EmailHook) admin.site.register(WebHook) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index b3ac583fd9d..ccc7bda44fc 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -22,7 +22,7 @@ from readthedocs.projects import constants from readthedocs.projects.exceptions import ProjectSpamError from readthedocs.projects.models import ( - Project, ProjectRelationship, EmailHook, WebHook, Domain) + Project, ProjectRelationship, EmailHook, WebHook, Domain, Feature) from readthedocs.redirects.models import Redirect @@ -595,3 +595,23 @@ class Meta(object): def __init__(self, *args, **kwargs): self.project = kwargs.pop('project', None) super(ProjectAdvertisingForm, self).__init__(*args, **kwargs) + + +class FeatureForm(forms.ModelForm): + + """Project feature form for dynamic admin choices + + This form converts the CharField into a ChoiceField on display. The + underlying driver won't attempt to do validation on the choices, and so we + can dynamically populate this list. + """ + + feature = forms.ChoiceField() + + class Meta(object): + model = Feature + fields = ['projects', 'feature', 'default_true'] + + def __init__(self, *args, **kwargs): + super(FeatureForm, self).__init__(*args, **kwargs) + self.fields['feature'].choices = Feature.FEATURES diff --git a/readthedocs/projects/migrations/0019_add-features.py b/readthedocs/projects/migrations/0019_add-features.py new file mode 100644 index 00000000000..6d1036dd123 --- /dev/null +++ b/readthedocs/projects/migrations/0019_add-features.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.12 on 2017-10-27 12:55 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0018_fix-translation-model'), + ] + + operations = [ + migrations.CreateModel( + name='Feature', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('feature_id', models.CharField(max_length=32, unique=True, verbose_name='Feature identifier')), + ('add_date', models.DateTimeField(auto_now_add=True, verbose_name='Date feature was added')), + ('default_true', models.BooleanField(default=False, verbose_name='Historical default is True')), + ], + ), + migrations.AddField( + model_name='feature', + name='projects', + field=models.ManyToManyField(blank=True, to='projects.Project'), + ), + ] diff --git a/readthedocs/projects/migrations/0020_add-api-project-proxy.py b/readthedocs/projects/migrations/0020_add-api-project-proxy.py new file mode 100644 index 00000000000..34eafa4846f --- /dev/null +++ b/readthedocs/projects/migrations/0020_add-api-project-proxy.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.12 on 2017-10-27 12:56 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0019_add-features'), + ] + + operations = [ + migrations.CreateModel( + name='APIProject', + fields=[ + ], + options={ + 'proxy': True, + }, + bases=('projects.project',), + ), + ] diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 9d898f7ddc9..86a63937d22 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -26,10 +26,10 @@ from readthedocs.projects.querysets import ( ProjectQuerySet, RelatedProjectQuerySet, - ChildRelatedProjectQuerySet + ChildRelatedProjectQuerySet, + FeatureQuerySet, ) from readthedocs.projects.templatetags.projects_tags import sort_version_aware -from readthedocs.projects.utils import make_api_version from readthedocs.projects.version_handling import determine_stable_version, version_windows from readthedocs.restapi.client import api from readthedocs.vcs_support.backends import backend_cls @@ -639,9 +639,10 @@ def get_latest_build(self, finished=True): return self.builds.filter(**kwargs).first() def api_versions(self): + from readthedocs.builds.models import APIVersion ret = [] for version_data in api.project(self.pk).active_versions.get()['versions']: - version = make_api_version(version_data) + version = APIVersion(**version_data) ret.append(version) return sort_version_aware(ret) @@ -821,6 +822,57 @@ def add_comment(self, version_slug, page, content_hash, commit, user, text): hash=content_hash, commit=commit) return node.comments.create(user=user, text=text) + @property + def features(self): + return Feature.objects.for_project(self) + + def has_feature(self, feature_id): + """Does project have existing feature flag + + If the feature has a historical True value before the feature was added, + we consider the project to have the flag. This is used for deprecating a + feature or changing behavior for new projects + """ + return self.features.filter(feature_id=feature_id).exists() + + +class APIProject(Project): + + """Project proxy model for API data deserialization + + This replaces the pattern where API data was deserialized into a mocked + :py:cls:`Project` object. This pattern was confusing, as it was not explicit + as to what form of object you were working with -- API backed or database + backed. + + This model preserves the Project model methods, allowing for overrides on + model field differences. This model pattern will generally only be used on + builder instances, where we are interacting solely with API data. + """ + + features = [] + + class Meta: + proxy = True + + def __init__(self, *args, **kwargs): + self.features = kwargs.pop('features', []) + # These fields only exist on the API return, not on the model, so we'll + # remove them to avoid throwing exceptions due to unexpected fields + for key in ['users', 'resource_uri', 'absolute_url', 'downloads', + 'main_language_project', 'related_projects']: + try: + del kwargs[key] + except KeyError: + pass + super(APIProject, self).__init__(*args, **kwargs) + + def save(self, *args, **kwargs): + return 0 + + def has_feature(self, feature_id): + return feature_id in self.features + @python_2_unicode_compatible class ImportedFile(models.Model): @@ -921,3 +973,63 @@ def delete(self, *args, **kwargs): # pylint: disable=arguments-differ from readthedocs.projects import tasks broadcast(type='app', task=tasks.symlink_domain, args=[self.project.pk, self.pk, True]) super(Domain, self).delete(*args, **kwargs) + + +@python_2_unicode_compatible +class Feature(models.Model): + + """Project feature flags + + Features should generally be added here as choices, however features may + also be added dynamically from a signal in other packages. Features can be + added by external packages with the use of signals:: + + @receiver(pre_init, sender=Feature) + def add_features(sender, **kwargs): + sender.FEATURES += (('blah', 'BLAH'),) + + The FeatureForm will grab the updated list on instantiation. + """ + + # Feature constants - this is not a exhaustive list of features, features + # may be added by other packages + USE_SPHINX_LATEST = 'use_sphinx_latest' + + FEATURES = ( + (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), + ) + + projects = models.ManyToManyField( + Project, + blank=True, + ) + # Feature is not implemented as a ChoiceField, as we don't want validation + # at the database level on this field. Arbitrary values are allowed here. + feature_id = models.CharField( + _('Feature identifier'), + max_length=32, + unique=True, + ) + add_date = models.DateTimeField( + _('Date feature was added'), + auto_now_add=True, + ) + default_true = models.BooleanField( + _('Historical default is True'), + default=False, + ) + + objects = FeatureQuerySet.as_manager() + + def __str__(self): + return "{0} feature".format( + self.get_feature_display(), + ) + + def get_feature_display(self): + """Implement display name field for fake ChoiceField + + Because the field is not a ChoiceField here, we need to manually + implement this behavior. + """ + return dict(self.FEATURES).get(self.feature_id, self.feature_id) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 5f7688d5529..241cd6c929d 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -3,6 +3,7 @@ from __future__ import absolute_import from django.db import models +from django.db.models import Q from guardian.shortcuts import get_objects_for_user from . import constants @@ -148,3 +149,13 @@ class ChildRelatedProjectQuerySetBase(RelatedProjectQuerySetBase): class ChildRelatedProjectQuerySet(SettingsOverrideObject): _default_class = ChildRelatedProjectQuerySetBase _override_setting = 'RELATED_PROJECT_MANAGER' + + +class FeatureQuerySet(models.QuerySet): + use_for_related_fields = True + + def for_project(self, project): + return self.filter( + Q(projects=project) | + Q(default_true=True, add_date__gt=project.pub_date) + ).distinct() diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 136af14b8e5..7d50ea0881c 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -29,14 +29,13 @@ from .exceptions import ProjectImportError from .models import ImportedFile, Project, Domain from .signals import before_vcs, after_vcs, before_build, after_build -from .utils import make_api_version, make_api_project from readthedocs.api.client import api as api_v1 from readthedocs.builds.constants import (LATEST, BUILD_STATE_CLONING, BUILD_STATE_INSTALLING, BUILD_STATE_BUILDING, BUILD_STATE_FINISHED) -from readthedocs.builds.models import Build, Version +from readthedocs.builds.models import Build, Version, APIVersion from readthedocs.builds.signals import build_complete from readthedocs.builds.syncers import Syncer from readthedocs.cdn.purge import purge @@ -49,6 +48,7 @@ from readthedocs.doc_builder.exceptions import BuildEnvironmentError from readthedocs.doc_builder.loader import get_builder_class from readthedocs.doc_builder.python_environments import Virtualenv, Conda +from readthedocs.projects.models import APIProject from readthedocs.restapi.client import api as api_v2 from readthedocs.restapi.utils import index_search_request from readthedocs.search.parse_json import process_all_json_files @@ -235,7 +235,7 @@ def run_build(self, docker=False, record=True): def get_project(project_pk): """Get project from API""" project_data = api_v2.project(project_pk).get() - project = make_api_project(project_data) + project = APIProject(**project_data) return project @staticmethod @@ -247,7 +247,7 @@ def get_version(project, version_pk): version_data = (api_v2 .version(project.slug) .get(slug=LATEST)['objects'][0]) - return make_api_version(version_data) + return APIVersion(**version_data) @staticmethod def get_build(build_pk): @@ -510,7 +510,7 @@ def update_imported_docs(version_pk): :param version_pk: Version id to update """ version_data = api_v2.version(version_pk).get() - version = make_api_version(version_data) + version = APIVersion(**version_data) project = version.project ret_dict = {} diff --git a/readthedocs/projects/utils.py b/readthedocs/projects/utils.py index df84d5c7cab..86e2040d03c 100644 --- a/readthedocs/projects/utils.py +++ b/readthedocs/projects/utils.py @@ -1,30 +1,31 @@ """Utility functions used by projects""" from __future__ import absolute_import -from builtins import object + import fnmatch +import logging import os import subprocess import traceback -import logging -from httplib2 import Http import redis import six +from builtins import object from django.conf import settings from django.core.cache import cache +from httplib2 import Http log = logging.getLogger(__name__) +# TODO make this a classmethod of Version def version_from_slug(slug, version): - from readthedocs.projects import tasks - from readthedocs.builds.models import Version + from readthedocs.builds.models import Version, APIVersion from readthedocs.restapi.client import api if getattr(settings, 'DONT_HIT_DB', True): version_data = api.version().get(project=slug, slug=version)['results'][0] - v = tasks.make_api_version(version_data) + v = APIVersion(**version_data) else: v = Version.objects.get(project__slug=slug, slug=version) return v @@ -167,36 +168,3 @@ class DictObj(object): def __getattr__(self, attr): return self.__dict__.get(attr) - - -# Prevent saving the temporary Project instance -def _new_save(*dummy_args, **dummy_kwargs): - log.warning("Called save on a non-real object.") - return 0 - - -def make_api_version(version_data): - """Make mock Version instance from API return""" - from readthedocs.builds.models import Version - for key in ['resource_uri', 'absolute_url', 'downloads']: - if key in version_data: - del version_data[key] - project_data = version_data['project'] - project = make_api_project(project_data) - version_data['project'] = project - ver = Version(**version_data) - ver.save = _new_save - - return ver - - -def make_api_project(project_data): - """Make mock Project instance from API return""" - from readthedocs.projects.models import Project - for key in ['users', 'resource_uri', 'absolute_url', 'downloads', - 'main_language_project', 'related_projects']: - if key in project_data: - del project_data[key] - project = Project(**project_data) - project.save = _new_save - return project diff --git a/readthedocs/restapi/serializers.py b/readthedocs/restapi/serializers.py index 1c43de42904..b85f2373136 100644 --- a/readthedocs/restapi/serializers.py +++ b/readthedocs/restapi/serializers.py @@ -1,13 +1,13 @@ """Defines serializers for each of our models.""" from __future__ import absolute_import -from builtins import object +from builtins import object from rest_framework import serializers from readthedocs.builds.models import Build, BuildCommandResult, Version -from readthedocs.projects.models import Project, Domain from readthedocs.oauth.models import RemoteOrganization, RemoteRepository +from readthedocs.projects.models import Project, Domain class ProjectSerializer(serializers.ModelSerializer): @@ -28,6 +28,18 @@ class Meta(object): class ProjectAdminSerializer(ProjectSerializer): + """Project serializer for admin only access + + Includes special internal fields that don't need to be exposed through the + general API, mostly for fields used in the build process + """ + + features = serializers.SlugRelatedField( + many=True, + read_only=True, + slug_field='feature_id', + ) + class Meta(ProjectSerializer.Meta): fields = ProjectSerializer.Meta.fields + ( 'enable_epub_build', @@ -44,6 +56,7 @@ class Meta(ProjectSerializer.Meta): 'skip', 'requirements_file', 'python_interpreter', + 'features', ) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index bc2e672f870..81d7b05eaaa 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -15,7 +15,7 @@ from readthedocs.builds.models import Build, Version from readthedocs.integrations.models import Integration -from readthedocs.projects.models import Project +from readthedocs.projects.models import Project, Feature from readthedocs.oauth.models import RemoteRepository, RemoteOrganization @@ -227,6 +227,40 @@ def test_ensure_get_unauth(self): resp = self.client.get("/api/v1/project/", data={"format": "json"}) self.assertEqual(resp.status_code, 200) + def test_project_features(self): + user = get(User, is_staff=True) + project = get(Project, main_language_project=None) + # One explicit, one implicit feature + feature1 = get(Feature, projects=[project]) + feature2 = get(Feature, projects=[], default_true=True) + feature3 = get(Feature, projects=[], default_true=False) + client = APIClient() + + client.force_authenticate(user=user) + resp = client.get('/api/v2/project/%s/' % (project.pk)) + self.assertEqual(resp.status_code, 200) + self.assertIn('features', resp.data) + self.assertEqual( + resp.data['features'], + [feature1.feature_id, feature2.feature_id] + ) + + def test_project_features_multiple_projects(self): + user = get(User, is_staff=True) + project1 = get(Project, main_language_project=None) + project2 = get(Project, main_language_project=None) + feature = get(Feature, projects=[project1, project2], default_true=True) + client = APIClient() + + client.force_authenticate(user=user) + resp = client.get('/api/v2/project/%s/' % (project1.pk)) + self.assertEqual(resp.status_code, 200) + self.assertIn('features', resp.data) + self.assertEqual( + resp.data['features'], + [feature.feature_id] + ) + class APIImportTests(TestCase): diff --git a/readthedocs/rtd_tests/tests/test_project_querysets.py b/readthedocs/rtd_tests/tests/test_project_querysets.py index 237af53c3d4..3772c6688ee 100644 --- a/readthedocs/rtd_tests/tests/test_project_querysets.py +++ b/readthedocs/rtd_tests/tests/test_project_querysets.py @@ -1,9 +1,11 @@ from __future__ import absolute_import +from datetime import timedelta + import django_dynamic_fixture as fixture from django.test import TestCase -from readthedocs.projects.models import Project, ProjectRelationship +from readthedocs.projects.models import Project, ProjectRelationship, Feature from readthedocs.projects.querysets import (ParentRelatedProjectQuerySet, ChildRelatedProjectQuerySet) @@ -27,3 +29,52 @@ def test_subproject_queryset_as_manager_gets_correct_class(self): mgr.__class__.__name__, 'ManagerFromParentRelatedProjectQuerySetBase' ) + + +class FeatureQuerySetTests(TestCase): + + def test_feature_for_project_is_explicit_applied(self): + project = fixture.get(Project, main_language_project=None) + feature = fixture.get(Feature, projects=[project]) + self.assertTrue(project.has_feature(feature.feature_id)) + + def test_feature_for_project_is_implicitly_applied(self): + project = fixture.get(Project, main_language_project=None) + # Explicit feature + feature1 = fixture.get(Feature, projects=[project]) + # False implicit feature + feature2 = fixture.get( + Feature, + projects=[], + add_date=project.pub_date + timedelta(days=1), + default_true=False, + ) + # True implicit feature before add date + feature3 = fixture.get( + Feature, + projects=[], + add_date=project.pub_date + timedelta(days=1), + default_true=True, + ) + # True implicit feature after add date + feature4 = fixture.get( + Feature, + projects=[], + add_date=project.pub_date - timedelta(days=1), + default_true=True, + ) + self.assertQuerysetEqual( + Feature.objects.for_project(project), + [repr(feature1), repr(feature3)], + ordered=False, + ) + + def test_feature_multiple_projects(self): + project1 = fixture.get(Project, main_language_project=None) + project2 = fixture.get(Project, main_language_project=None) + feature = fixture.get(Feature, projects=[project1, project2]) + self.assertQuerysetEqual( + Feature.objects.for_project(project1), + [repr(feature)], + ordered=False, + )