Skip to content

Commit

Permalink
Build details page: normalize/trim command paths (second attempt)
Browse files Browse the repository at this point in the history
Continuation of the work done in
#9815 that didn't work as we
expected.

We weren't saving the commands properly when hitting the API since
`serializers.SerializerMethodField` is a read-only field. This commit fixes this
problem by using a different Serializer depending if it's a GET request or not,
and if it's made by an Admin or not.

It overrides the method `BuildViewSet.get_serializer_class` to manage these
different cases and return the proper serializer.

Done on top of #9815 to be sure the
tests for saving commands are passing.
  • Loading branch information
humitos committed Dec 21, 2022
1 parent 3d2dfa0 commit 7c8c4f3
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 3 deletions.
49 changes: 48 additions & 1 deletion readthedocs/api/v2/serializers.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -133,11 +136,49 @@ class Meta:
exclude = []


class BuildCommandUISerializer(BuildCommandSerializer):
"""
Serializer used on GETs to trimm 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/
<container_hash>/<project_slug>/envs/<version_slug>/bin/python
$ /home/docs/checkouts/readthedocs.org/user_builds/
<project_slug>/envs/<version_slug>/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."""

commands = BuildCommandSerializer(many=True, read_only=True)
commands = BuildCommandUISerializer(many=True, read_only=True)
project_slug = serializers.ReadOnlyField(source='project.slug')
version_slug = serializers.ReadOnlyField(source='get_version_slug')
docs_url = serializers.SerializerMethodField()
Expand All @@ -162,11 +203,17 @@ class BuildAdminSerializer(BuildSerializer):

"""Build serializer for display to admin users and build instances."""

commands = BuildCommandSerializer(many=True, read_only=True)

class Meta(BuildSerializer.Meta):
# `_config` should be excluded to avoid conflicts with `config`
exclude = ('_config',)


class BuildAdminUISerializer(BuildAdminSerializer):
commands = BuildCommandUISerializer(many=True, read_only=True)


class SearchIndexSerializer(serializers.Serializer):
q = serializers.CharField(max_length=500)
project = serializers.CharField(max_length=500, required=False)
Expand Down
19 changes: 17 additions & 2 deletions readthedocs/api/v2/views/model_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from ..permissions import APIPermission, APIRestrictedPermission, IsOwner
from ..serializers import (
BuildAdminSerializer,
BuildAdminUISerializer,
BuildCommandSerializer,
BuildSerializer,
DomainSerializer,
Expand Down Expand Up @@ -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 check for GET method
if self.request.method == "GET":
return BuildAdminUISerializer
return BuildAdminSerializer
return BuildSerializer

@decorators.action(
detail=False,
permission_classes=[permissions.IsAdminUser],
Expand Down

0 comments on commit 7c8c4f3

Please sign in to comment.