Merge pull request #3054 from rtfd/ginstrom-migrate-v1-to-v2

Remove apiv1 from our internal usage.

Author: ericholscher

readthedocs/doc_builder/environments.py

@@ -22,7 +22,6 @@
 from readthedocs.builds.constants import BUILD_STATE_FINISHED
 from readthedocs.builds.models import BuildCommandResultMixin
 from readthedocs.projects.constants import LOG_TEMPLATE
-from readthedocs.api.client import api as api_v1
 from readthedocs.restapi.client import api as api_v2
 
 from .exceptions import (BuildEnvironmentException, BuildEnvironmentError,
@@ -37,7 +36,7 @@
 
 
 __all__ = (
-    'api_v1', 'api_v2',
+    'api_v2',
     'BuildCommand', 'DockerBuildCommand',
     'BuildEnvironment', 'LocalEnvironment', 'DockerEnvironment',
 )

readthedocs/projects/models.py

@@ -17,7 +17,6 @@
 from guardian.shortcuts import assign
 from taggit.managers import TaggableManager
 
-from readthedocs.api.client import api
 from readthedocs.builds.constants import LATEST, LATEST_VERBOSE_NAME, STABLE
 from readthedocs.core.resolver import resolve, resolve_domain
 from readthedocs.core.utils import broadcast, slugify
@@ -32,7 +31,7 @@
 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 as apiv2
+from readthedocs.restapi.client import api
 from readthedocs.vcs_support.backends import backend_cls
 from readthedocs.vcs_support.base import VCSProject
 from readthedocs.vcs_support.utils import Lock, NonBlockingLock
@@ -357,7 +356,7 @@ def get_builds_url(self):
 
     def get_canonical_url(self):
         if getattr(settings, 'DONT_HIT_DB', True):
-            return apiv2.project(self.pk).canonical_url().get()['url']
+            return api.project(self.pk).canonical_url().get()['url']
         return self.get_docs_url()
 
     def get_subproject_urls(self):
@@ -368,7 +367,7 @@ def get_subproject_urls(self):
         if getattr(settings, 'DONT_HIT_DB', True):
             return [(proj['slug'], proj['canonical_url'])
                     for proj in (
-                        apiv2.project(self.pk)
+                        api.project(self.pk)
                         .subprojects()
                         .get()['subprojects'])]
         return [(proj.child.slug, proj.child.get_docs_url())
@@ -641,8 +640,7 @@ def get_latest_build(self, finished=True):
 
     def api_versions(self):
         ret = []
-        for version_data in api.version.get(project=self.pk,
-                                            active=True)['objects']:
+        for version_data in api.project(self.pk).active_versions.get()['versions']:
             version = make_api_version(version_data)
             ret.append(version)
         return sort_version_aware(ret)

readthedocs/projects/tasks.py

@@ -234,17 +234,17 @@ def run_build(self, docker=False, record=True):
     @staticmethod
     def get_project(project_pk):
         """Get project from API"""
-        project_data = api_v1.project(project_pk).get()
+        project_data = api_v2.project(project_pk).get()
         project = make_api_project(project_data)
         return project
 
     @staticmethod
     def get_version(project, version_pk):
         """Ensure we're using a sane version"""
         if version_pk:
-            version_data = api_v1.version(version_pk).get()
+            version_data = api_v2.version(version_pk).get()
         else:
-            version_data = (api_v1
+            version_data = (api_v2
                             .version(project.slug)
                             .get(slug=LATEST)['objects'][0])
         return make_api_version(version_data)
@@ -509,7 +509,7 @@ def update_imported_docs(version_pk):
 
     :param version_pk: Version id to update
     """
-    version_data = api_v1.version(version_pk).get()
+    version_data = api_v2.version(version_pk).get()
     version = make_api_version(version_data)
     project = version.project
     ret_dict = {}

readthedocs/restapi/serializers.py

@@ -26,6 +26,27 @@ class Meta(object):
         )
 
 
+class ProjectAdminSerializer(ProjectSerializer):
+
+    class Meta(ProjectSerializer.Meta):
+        fields = ProjectSerializer.Meta.fields + (
+            'enable_epub_build',
+            'enable_pdf_build',
+            'conf_py_file',
+            'analytics_code',
+            'cdn_enabled',
+            'container_image',
+            'container_mem_limit',
+            'container_time_limit',
+            'install_project',
+            'use_system_packages',
+            'suffix',
+            'skip',
+            'requirements_file',
+            'python_interpreter',
+        )
+
+
 class VersionSerializer(serializers.ModelSerializer):
     project = ProjectSerializer()
     downloads = serializers.DictField(source='get_downloads', read_only=True)
@@ -41,7 +62,15 @@ class Meta(object):
         )
 
 
+class VersionAdminSerializer(VersionSerializer):
+
+    """Version serializer that returns admin project data"""
+
+    project = ProjectAdminSerializer()
+
+
 class BuildCommandSerializer(serializers.ModelSerializer):
+
     run_time = serializers.ReadOnlyField()
 
     class Meta(object):
@@ -51,7 +80,7 @@ class Meta(object):
 
 class BuildSerializer(serializers.ModelSerializer):
 
-    """Readonly version of the build serializer, used for user facing display"""
+    """Build serializer for user display, doesn't display internal fields"""
 
     commands = BuildCommandSerializer(many=True, read_only=True)
     state_display = serializers.ReadOnlyField(source='get_state_display')
@@ -61,13 +90,12 @@ class Meta(object):
         exclude = ('builder',)
 
 
-class BuildSerializerFull(BuildSerializer):
+class BuildAdminSerializer(BuildSerializer):
 
-    """Writeable Build instance serializer, for admin access by builders"""
+    """Build serializer for display to admin users and build instances"""
 
-    class Meta(object):
-        model = Build
-        exclude = ('')
+    class Meta(BuildSerializer.Meta):
+        exclude = ()
 
 
 class SearchIndexSerializer(serializers.Serializer):

readthedocs/restapi/utils.py

@@ -115,9 +115,8 @@ def index_search_request(version, page_list, commit, project_scale, page_scale,
     routes.extend([p.parent.slug for p in project.superprojects.all()])
     for page in page_list:
         log.debug("Indexing page: %s:%s", project.slug, page['path'])
-        page_id = (hashlib
-                   .md5('-'.join([project.slug, version.slug, page['path']]))
-                   .hexdigest())
+        to_hash = '-'.join([project.slug, version.slug, page['path']])
+        page_id = hashlib.md5(to_hash.encode('utf-8')).hexdigest()
         index_list.append({
             'id': page_id,
             'project': project.slug,
@@ -132,11 +131,10 @@ def index_search_request(version, page_list, commit, project_scale, page_scale,
         })
         if section:
             for sect in page['sections']:
+                id_to_hash = '-'.join([project.slug, version.slug,
+                                       page['path'], sect['id']])
                 section_index_list.append({
-                    'id': (hashlib
-                           .md5('-'.join([project.slug, version.slug,
-                                         page['path'], sect['id']]))
-                           .hexdigest()),
+                    'id': (hashlib.md5(id_to_hash.encode('utf-8')).hexdigest()),
                     'project': project.slug,
                     'version': version.slug,
                     'path': page['path'],

readthedocs/restapi/views/model_views.py

@@ -21,31 +21,52 @@
 
 from ..permissions import (APIPermission, APIRestrictedPermission,
                            RelatedProjectIsOwner, IsOwner)
-from ..serializers import (BuildSerializerFull, BuildSerializer,
-                           BuildCommandSerializer, ProjectSerializer,
-                           VersionSerializer, DomainSerializer,
-                           RemoteOrganizationSerializer,
+from ..serializers import (BuildSerializer, BuildAdminSerializer,
+                           BuildCommandSerializer,
+                           ProjectSerializer, ProjectAdminSerializer,
+                           VersionSerializer, VersionAdminSerializer,
+                           DomainSerializer, RemoteOrganizationSerializer,
                            RemoteRepositorySerializer)
 from .. import utils as api_utils
 
 log = logging.getLogger(__name__)
 
 
-class ProjectViewSet(viewsets.ModelViewSet):
+class UserSelectViewSet(viewsets.ModelViewSet):
+
+    """View set that varies serializer class based on request user credentials
+
+    Viewsets using this class should have an attribute `admin_serializer_class`,
+    which is a serializer that might have more fields that only admin/staff
+    users require. If the user is staff, this class will be returned instead.
+    """
+
+    def get_serializer_class(self):
+        try:
+            if self.request.user.is_staff and self.admin_serializer_class is not None:
+                return self.admin_serializer_class
+        except AttributeError:
+            pass
+        return self.serializer_class
+
+    def get_queryset(self):
+        """Use our API manager method to determine authorization on queryset"""
+        return self.model.objects.api(self.request.user)
+
+
+class ProjectViewSet(UserSelectViewSet):
 
     """List, filter, etc. Projects."""
 
     permission_classes = [APIPermission]
     renderer_classes = (JSONRenderer,)
     serializer_class = ProjectSerializer
+    admin_serializer_class = ProjectAdminSerializer
     model = Project
     paginate_by = 100
     paginate_by_param = 'page_size'
     max_paginate_by = 1000
 
-    def get_queryset(self):
-        return self.model.objects.api(self.request.user)
-
     @decorators.detail_route()
     def valid_versions(self, request, **kwargs):
         """Maintain state of versions that are wanted."""
@@ -81,6 +102,15 @@ def subprojects(self, request, **kwargs):
             'subprojects': ProjectSerializer(children, many=True).data
         })
 
+    @detail_route()
+    def active_versions(self, request, **kwargs):
+        project = get_object_or_404(
+            Project.objects.api(request.user), pk=kwargs['pk'])
+        versions = project.versions.filter(active=True)
+        return Response({
+            'versions': VersionSerializer(versions, many=True).data
+        })
+
     @decorators.detail_route(permission_classes=[permissions.IsAdminUser])
     def token(self, request, **kwargs):
         project = get_object_or_404(
@@ -157,35 +187,22 @@ def sync_versions(self, request, **kwargs):
         })
 
 
-class VersionViewSet(viewsets.ModelViewSet):
+class VersionViewSet(UserSelectViewSet):
 
     permission_classes = [APIRestrictedPermission]
     renderer_classes = (JSONRenderer,)
     serializer_class = VersionSerializer
+    admin_serializer_class = VersionAdminSerializer
     model = Version
 
-    def get_queryset(self):
-        return self.model.objects.api(self.request.user)
-
 
-class BuildViewSetBase(viewsets.ModelViewSet):
+class BuildViewSetBase(UserSelectViewSet):
     permission_classes = [APIRestrictedPermission]
     renderer_classes = (JSONRenderer,)
+    serializer_class = BuildSerializer
+    admin_serializer_class = BuildAdminSerializer
     model = Build
 
-    def get_queryset(self):
-        return self.model.objects.api(self.request.user)
-
-    def get_serializer_class(self):
-        """Vary serializer class based on user status
-
-        This is used to allow write to write-only fields on Build by admin users
-        and to not return those fields to non-admin users.
-        """
-        if self.request.user.is_staff:
-            return BuildSerializerFull
-        return BuildSerializer
-
 
 class BuildViewSet(SettingsOverrideObject):
 
@@ -194,18 +211,12 @@ class BuildViewSet(SettingsOverrideObject):
     _default_class = BuildViewSetBase
 
 
-class BuildCommandViewSet(viewsets.ModelViewSet):
-
-    """This is currently a write-only way to update the commands on build."""
-
+class BuildCommandViewSet(UserSelectViewSet):
     permission_classes = [APIRestrictedPermission]
     renderer_classes = (JSONRenderer,)
     serializer_class = BuildCommandSerializer
     model = BuildCommandResult
 
-    def get_queryset(self):
-        return self.model.objects.api(self.request.user)
-
 
 class NotificationViewSet(viewsets.ReadOnlyModelViewSet):
     permission_classes = (permissions.IsAuthenticated, RelatedProjectIsOwner)
@@ -216,15 +227,12 @@ def get_queryset(self):
         return self.model.objects.api(self.request.user)
 
 
-class DomainViewSet(viewsets.ModelViewSet):
+class DomainViewSet(UserSelectViewSet):
     permission_classes = [APIRestrictedPermission]
     renderer_classes = (JSONRenderer,)
     serializer_class = DomainSerializer
     model = Domain
 
-    def get_queryset(self):
-        return self.model.objects.api(self.request.user)
-
 
 class RemoteOrganizationViewSet(viewsets.ReadOnlyModelViewSet):
     permission_classes = [IsOwner]

readthedocs/rtd_tests/mocks/mock_api.py

@@ -96,7 +96,5 @@ def mock_api(repo):
     with mock.patch('readthedocs.restapi.client.api', api_mock), \
             mock.patch('readthedocs.api.client.api', api_mock), \
             mock.patch('readthedocs.projects.tasks.api_v2', api_mock), \
-            mock.patch('readthedocs.projects.tasks.api_v1', api_mock), \
-            mock.patch('readthedocs.doc_builder.environments.api_v1', api_mock), \
             mock.patch('readthedocs.doc_builder.environments.api_v2', api_mock):
         yield api_mock

readthedocs/rtd_tests/tests/test_api.py

@@ -1,10 +1,11 @@
 from __future__ import absolute_import
-from builtins import str
+
 import json
 import base64
 import datetime
 
 import mock
+from builtins import str
 from django.test import TestCase
 from django.contrib.auth.models import User
 from django_dynamic_fixture import get
@@ -168,6 +169,23 @@ def test_make_project(self):
         obj = json.loads(resp.content)
         self.assertEqual(obj['slug'], 'awesome-project')
 
+    def test_user_doesnt_get_full_api_return(self):
+        user_normal = get(User, is_staff=False)
+        user_admin = get(User, is_staff=True)
+        project = get(Project, main_language_project=None, conf_py_file='foo')
+        client = APIClient()
+
+        client.force_authenticate(user=user_normal)
+        resp = client.get('/api/v2/project/%s/' % (project.pk))
+        self.assertEqual(resp.status_code, 200)
+        self.assertNotIn('conf_py_file', resp.data)
+
+        client.force_authenticate(user=user_admin)
+        resp = client.get('/api/v2/project/%s/' % (project.pk))
+        self.assertEqual(resp.status_code, 200)
+        self.assertIn('conf_py_file', resp.data)
+        self.assertEqual(resp.data['conf_py_file'], 'foo')
+
     def test_invalid_make_project(self):
         """
         Test that the authentication is turned on.