Add feature flipping models to Projects, plus example feature flip (#3194)

* Add feature flipping models to Projects, plus example feature flip

Also, use a new pattern for API instance objects

* Refactoring the make_api_* calls into the API models

* Add migrations for proxy models

* Add some admin features

* Rework feature relationship, add default true value based on date

This allows for features on deprecated code, where we can say the feature is
true historically.

* Fix some issues, more tests

* Rename Feature.feature -> Feature.feature_id, other cleanup

* Use semver instead

Author: agjohnson

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',),
+        ),
+    ]

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):
 

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)

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',

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)

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

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'),
+        ),
+    ]

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',),
+        ),
+    ]

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)