diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index 92977f5796b..3fa97b0d036 100644 --- a/readthedocs/doc_builder/environments.py +++ b/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', ) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index ea13d931abe..9d898f7ddc9 100644 --- a/readthedocs/projects/models.py +++ b/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) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 660936c6b60..136af14b8e5 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -234,7 +234,7 @@ 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 @@ -242,9 +242,9 @@ def get_project(project_pk): 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 = {} diff --git a/readthedocs/restapi/serializers.py b/readthedocs/restapi/serializers.py index 83a12f2d65c..1c43de42904 100644 --- a/readthedocs/restapi/serializers.py +++ b/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): diff --git a/readthedocs/restapi/utils.py b/readthedocs/restapi/utils.py index 5b719c7c104..41bae9b4236 100644 --- a/readthedocs/restapi/utils.py +++ b/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'], diff --git a/readthedocs/restapi/views/model_views.py b/readthedocs/restapi/views/model_views.py index 2cd26a655f0..a4e997d4097 100644 --- a/readthedocs/restapi/views/model_views.py +++ b/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] diff --git a/readthedocs/rtd_tests/mocks/mock_api.py b/readthedocs/rtd_tests/mocks/mock_api.py index acf01178557..ad1cdab065b 100644 --- a/readthedocs/rtd_tests/mocks/mock_api.py +++ b/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 diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index a315545734d..bc2e672f870 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/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. diff --git a/readthedocs/rtd_tests/tests/test_builds.py b/readthedocs/rtd_tests/tests/test_builds.py index b26ffa5f0c6..19f383aa3e8 100644 --- a/readthedocs/rtd_tests/tests/test_builds.py +++ b/readthedocs/rtd_tests/tests/test_builds.py @@ -157,10 +157,6 @@ def test_builder_no_comments(self): def test_build_pdf_latex_failures(self): '''Build failure if latex fails''' - if six.PY3: - import pytest - pytest.xfail( - "test_build_pdf_latex_failures is known to fail on 3.6") self.mocks.patches['html_build'].stop() self.mocks.patches['pdf_build'].stop() @@ -183,11 +179,11 @@ def test_build_pdf_latex_failures(self): # Mock out the separate calls to Popen using an iterable side_effect returns = [ - (('', ''), 0), # sphinx-build html - (('', ''), 0), # sphinx-build pdf - (('', ''), 1), # latex - (('', ''), 0), # makeindex - (('', ''), 0), # latex + ((b'', b''), 0), # sphinx-build html + ((b'', b''), 0), # sphinx-build pdf + ((b'', b''), 1), # latex + ((b'', b''), 0), # makeindex + ((b'', b''), 0), # latex ] mock_obj = mock.Mock() mock_obj.communicate.side_effect = [output for (output, status) @@ -203,10 +199,6 @@ def test_build_pdf_latex_failures(self): def test_build_pdf_latex_not_failure(self): '''Test pass during PDF builds and bad latex failure status code''' - if six.PY3: - import pytest - pytest.xfail( - "test_build_pdf_latex_not_failure is known to fail on 3.6") self.mocks.patches['html_build'].stop() self.mocks.patches['pdf_build'].stop() @@ -229,11 +221,11 @@ def test_build_pdf_latex_not_failure(self): # Mock out the separate calls to Popen using an iterable side_effect returns = [ - (('', ''), 0), # sphinx-build html - (('', ''), 0), # sphinx-build pdf - (('Output written on foo.pdf', ''), 1), # latex - (('', ''), 0), # makeindex - (('', ''), 0), # latex + ((b'', b''), 0), # sphinx-build html + ((b'', b''), 0), # sphinx-build pdf + ((b'Output written on foo.pdf', b''), 1), # latex + ((b'', b''), 0), # makeindex + ((b'', b''), 0), # latex ] mock_obj = mock.Mock() mock_obj.communicate.side_effect = [output for (output, status) diff --git a/readthedocs/rtd_tests/tests/test_config_wrapper.py b/readthedocs/rtd_tests/tests/test_config_wrapper.py index 3aea4c836a3..aaa374ae7ce 100644 --- a/readthedocs/rtd_tests/tests/test_config_wrapper.py +++ b/readthedocs/rtd_tests/tests/test_config_wrapper.py @@ -3,12 +3,10 @@ from django.test import TestCase from django_dynamic_fixture import get -import six - from readthedocs_build.config import BuildConfig, ProjectConfig, InvalidConfig from readthedocs.builds.models import Version from readthedocs.projects.models import Project -from readthedocs.doc_builder.config import ConfigWrapper, load_yaml_config +from readthedocs.doc_builder.config import load_yaml_config def create_load(config=None): @@ -128,7 +126,7 @@ def test_python_invalid_version_in_config(self, load_config): self.project.container_image = 'readthedocs/build:2.0' self.project.save() with self.assertRaises(InvalidConfig): - config = load_yaml_config(self.version) + load_yaml_config(self.version) def test_install_project(self, load_config): load_config.side_effect = create_load() @@ -189,11 +187,7 @@ def test_conda(self, load_config): self.assertEqual(config.conda_file, None) def test_requirements_file(self, load_config): - if six.PY3: - import pytest - pytest.xfail("test_requirements_file is known to fail on 3.6") - - requirements_file = 'wsgi.py' if six.PY2 else 'readthedocs/wsgi.py' + requirements_file = 'wsgi.py' load_config.side_effect = create_load({ 'requirements_file': requirements_file }) diff --git a/readthedocs/settings/dev.py b/readthedocs/settings/dev.py index c0e436c6a35..4ae0ca0295e 100644 --- a/readthedocs/settings/dev.py +++ b/readthedocs/settings/dev.py @@ -29,6 +29,7 @@ def DATABASES(self): # noqa SLUMBER_USERNAME = 'test' SLUMBER_PASSWORD = 'test' # noqa: ignore dodgy check SLUMBER_API_HOST = 'http://localhost:8000' + PUBLIC_API_URL = 'http://localhost:8000' BROKER_URL = 'redis://localhost:6379/0' CELERY_RESULT_BACKEND = 'redis://localhost:6379/0' diff --git a/readthedocs/vcs_support/base.py b/readthedocs/vcs_support/base.py index be1500f20ed..3416de0ff11 100644 --- a/readthedocs/vcs_support/base.py +++ b/readthedocs/vcs_support/base.py @@ -71,7 +71,10 @@ def run(self, *args): except UnicodeDecodeError: # >:x pass - return (process.returncode, stdout, stderr) + return ( + process.returncode, + stdout.decode('utf-8'), + stderr.decode('utf-8')) @property def env(self):