diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py index dd1a864028d..37c58e06c6d 100644 --- a/readthedocs/api/v2/serializers.py +++ b/readthedocs/api/v2/serializers.py @@ -1,6 +1,9 @@ """Defines serializers for each of our models.""" +import re + from allauth.socialaccount.models import SocialAccount +from django.conf import settings from rest_framework import serializers from readthedocs.builds.models import Build, BuildCommandResult, Version @@ -133,11 +136,63 @@ class Meta: exclude = [] +class BuildCommandReadOnlySerializer(BuildCommandSerializer): + + """ + Serializer used on GETs to trim the commands' path. + + Remove unreadable paths from the command outputs when returning it from the API. + We could make this change at build level, but we want to avoid undoable issues from now + and hack a small solution to fix the immediate problem. + + This converts: + $ /usr/src/app/checkouts/readthedocs.org/user_builds/ + //envs//bin/python + $ /home/docs/checkouts/readthedocs.org/user_builds/ + /envs//bin/python + into + $ python + """ + + command = serializers.SerializerMethodField() + + def get_command(self, obj): + project_slug = obj.build.version.project.slug + version_slug = obj.build.version.slug + docroot = settings.DOCROOT.rstrip("/") # remove trailing '/' + + # Remove Docker hash from DOCROOT when running it locally + # DOCROOT contains the Docker container hash (e.g. b7703d1b5854). + # We have to remove it from the DOCROOT it self since it changes each time + # we spin up a new Docker instance locally. + container_hash = "/" + if settings.RTD_DOCKER_COMPOSE: + docroot = re.sub("/[0-9a-z]+/?$", "", settings.DOCROOT, count=1) + container_hash = "/([0-9a-z]+/)?" + + regex = f"{docroot}{container_hash}{project_slug}/envs/{version_slug}(/bin/)?" + command = re.sub(regex, "", obj.command, count=1) + return command + + class BuildSerializer(serializers.ModelSerializer): - """Build serializer for user display, doesn't display internal fields.""" + """ + Build serializer for user display. - commands = BuildCommandSerializer(many=True, read_only=True) + This is the default serializer for Build objects over read-only operations from regular users. + Take into account that: + + - It doesn't display internal fields (builder, _config) + - It's read-only for multiple fields (commands, project_slug, etc) + + Staff users should use either: + + - BuildAdminSerializer for write operations (e.g. builders hitting the API), + - BuildAdminReadOnlySerializer for read-only actions (e.g. dashboard retrieving build details) + """ + + commands = BuildCommandReadOnlySerializer(many=True, read_only=True) project_slug = serializers.ReadOnlyField(source='project.slug') version_slug = serializers.ReadOnlyField(source='get_version_slug') docs_url = serializers.SerializerMethodField() @@ -160,13 +215,32 @@ def get_docs_url(self, obj): class BuildAdminSerializer(BuildSerializer): - """Build serializer for display to admin users and build instances.""" + """ + Build serializer to update Build objects from build instances. + + It allows write operations on `commands` and display fields (e.g. builder) + that are allowed for admin purposes only. + """ + + commands = BuildCommandSerializer(many=True, read_only=True) class Meta(BuildSerializer.Meta): # `_config` should be excluded to avoid conflicts with `config` exclude = ('_config',) +class BuildAdminReadOnlySerializer(BuildAdminSerializer): + + """ + Build serializer to retrieve Build objects from the dashboard. + + It uses `BuildCommandReadOnlySerializer` to automatically parse the command + and trim the useless path. + """ + + commands = BuildCommandReadOnlySerializer(many=True, read_only=True) + + class SearchIndexSerializer(serializers.Serializer): q = serializers.CharField(max_length=500) project = serializers.CharField(max_length=500, required=False) diff --git a/readthedocs/api/v2/views/model_views.py b/readthedocs/api/v2/views/model_views.py index b8b3eb21cfa..1faf7dc2066 100644 --- a/readthedocs/api/v2/views/model_views.py +++ b/readthedocs/api/v2/views/model_views.py @@ -22,6 +22,7 @@ from ..permissions import APIPermission, APIRestrictedPermission, IsOwner from ..serializers import ( + BuildAdminReadOnlySerializer, BuildAdminSerializer, BuildCommandSerializer, BuildSerializer, @@ -224,11 +225,25 @@ class VersionViewSet(DisableListEndpoint, UserSelectViewSet): class BuildViewSet(DisableListEndpoint, UserSelectViewSet): permission_classes = [APIRestrictedPermission] renderer_classes = (JSONRenderer, PlainTextBuildRenderer) - serializer_class = BuildSerializer - admin_serializer_class = BuildAdminSerializer model = Build filterset_fields = ('project__slug', 'commit') + def get_serializer_class(self): + """ + Return the proper serializer for UI and Admin. + + This ViewSet has a sligtly different pattern since we want to + pre-process the `command` field before returning it to the user, and we + also want to have a specific serializer for admins. + """ + if self.request.user.is_staff: + # Logic copied from `UserSelectViewSet.get_serializer_class` + # and extended to choose serializer from self.action + if self.action not in ["list", "retrieve"]: + return BuildAdminSerializer # Staff write-only + return BuildAdminReadOnlySerializer # Staff read-only + return BuildSerializer # Non-staff + @decorators.action( detail=False, permission_classes=[permissions.IsAdminUser], diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index f00bd731d01..4a20cd48e2a 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -3,11 +3,13 @@ import json from unittest import mock +import dateutil from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import User from django.http import QueryDict from django.test import TestCase, override_settings from django.urls import reverse +from django.utils import timezone from django_dynamic_fixture import get from rest_framework import status from rest_framework.test import APIClient @@ -474,16 +476,18 @@ def test_make_build_commands(self): ) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) build = resp.data - now = datetime.datetime.utcnow() + now = timezone.now() + start_time = now - datetime.timedelta(seconds=5) + end_time = now resp = client.post( '/api/v2/command/', { - 'build': build['id'], - 'command': 'echo test', - 'description': 'foo', - 'exit_code': 0, - 'start_time': str(now - datetime.timedelta(seconds=5)), - 'end_time': str(now), + "build": build["id"], + "command": "echo test", + "description": "foo", + "exit_code": 0, + "start_time": start_time, + "end_time": end_time, }, format='json', ) @@ -491,9 +495,17 @@ def test_make_build_commands(self): resp = client.get('/api/v2/build/%s/' % build['id']) self.assertEqual(resp.status_code, 200) build = resp.data - self.assertEqual(len(build['commands']), 1) - self.assertEqual(build['commands'][0]['run_time'], 5) - self.assertEqual(build['commands'][0]['description'], 'foo') + self.assertEqual(len(build["commands"]), 1) + self.assertEqual(build["commands"][0]["command"], "echo test") + self.assertEqual(build["commands"][0]["run_time"], 5) + self.assertEqual(build["commands"][0]["description"], "foo") + self.assertEqual(build["commands"][0]["exit_code"], 0) + self.assertEqual( + dateutil.parser.parse(build["commands"][0]["start_time"]), start_time + ) + self.assertEqual( + dateutil.parser.parse(build["commands"][0]["end_time"]), end_time + ) def test_get_raw_log_success(self): project = Project.objects.get(pk=1)