From b60813ea6b625331fb3bddd436c569c8eb166b4b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 9 Dec 2024 16:48:26 +0100 Subject: [PATCH 1/4] APIv3: proxy these URLs to be served from El Proxito `/_/api/v3/` * Related: https://github.com/readthedocs/addons/issues/356 * Related: https://github.com/readthedocs/addons/pull/468 --- readthedocs/api/v3/proxied_urls.py | 3 +++ readthedocs/api/v3/views.py | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/readthedocs/api/v3/proxied_urls.py b/readthedocs/api/v3/proxied_urls.py index 28a3ee75b76..09d5d77a782 100644 --- a/readthedocs/api/v3/proxied_urls.py +++ b/readthedocs/api/v3/proxied_urls.py @@ -10,9 +10,12 @@ from readthedocs.api.v3.proxied_views import ProxiedEmbedAPI from readthedocs.search.api.v3.views import ProxiedSearchAPI +from .urls import router + api_proxied_urls = [ path("embed/", ProxiedEmbedAPI.as_view(), name="embed_api_v3"), path("search/", ProxiedSearchAPI.as_view(), name="search_api_v3"), ] urlpatterns = api_proxied_urls +urlpatterns += router.urls diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 5376a081f4c..3ccb06af3af 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -19,7 +19,6 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.renderers import BrowsableAPIRenderer from rest_framework.response import Response -from rest_framework.throttling import AnonRateThrottle, UserRateThrottle from rest_framework.viewsets import GenericViewSet, ModelViewSet, ReadOnlyModelViewSet from rest_framework_extensions.mixins import NestedViewSetMixin @@ -110,7 +109,7 @@ class APIv3Settings: LimitOffsetPagination.default_limit = 10 renderer_classes = (AlphabeticalSortedJSONRenderer, BrowsableAPIRenderer) - throttle_classes = (UserRateThrottle, AnonRateThrottle) + # throttle_classes = (UserRateThrottle, AnonRateThrottle) filter_backends = (filters.DjangoFilterBackend,) metadata_class = SimpleMetadata From 3306c4d8b71f30d04961af6e773828dc4a73ec1f Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 12 Dec 2024 18:49:45 +0100 Subject: [PATCH 2/4] Move `filetreediff` addons response to its own endpoint --- readthedocs/api/v3/urls.py | 12 +++ readthedocs/api/v3/views.py | 122 +++++++++++++++++++++++++ readthedocs/proxito/views/hosting.py | 128 +-------------------------- 3 files changed, 135 insertions(+), 127 deletions(-) diff --git a/readthedocs/api/v3/urls.py b/readthedocs/api/v3/urls.py index f20cc6705a7..7e184066a23 100644 --- a/readthedocs/api/v3/urls.py +++ b/readthedocs/api/v3/urls.py @@ -3,6 +3,7 @@ BuildsCreateViewSet, BuildsViewSet, EnvironmentVariablesViewSet, + FileTreeDiffViewSet, NotificationsBuildViewSet, NotificationsForUserViewSet, NotificationsOrganizationViewSet, @@ -77,6 +78,17 @@ ], ) +# allows /api/v3/projects/pip/versions/v3.6.2/filetreediff/ +versions.register( + r"filetreediff", + FileTreeDiffViewSet, + basename="projects-versions-filetreediff", + parents_query_lookups=[ + "project__slug", + "version__slug", + ], +) + # allows /api/v3/projects/pip/builds/ # allows /api/v3/projects/pip/builds/1053/ builds = projects.register( diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 3ccb06af3af..46392adf511 100644 --- a/readthedocs/api/v3/views.py +++ b/readthedocs/api/v3/views.py @@ -25,9 +25,11 @@ from readthedocs.api.v2.permissions import ReadOnlyPermission from readthedocs.builds.constants import EXTERNAL from readthedocs.builds.models import Build, Version +from readthedocs.core.resolver import Resolver from readthedocs.core.utils import trigger_build from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.core.views.hooks import trigger_sync_versions +from readthedocs.filetreediff import get_diff from readthedocs.notifications.models import Notification from readthedocs.oauth.models import ( RemoteOrganization, @@ -381,6 +383,126 @@ def get_queryset(self): return super().get_queryset().exclude(type=EXTERNAL) +class FileTreeDiffViewSet( + APIv3Settings, + NestedViewSetMixin, + ProjectQuerySetMixin, + FlexFieldsMixin, + ReadOnlyModelViewSet, +): + model = Version + lookup_field = "slug" + lookup_url_kwarg = "version_slug" + + # Allow ``.`` (dots) on version slug + lookup_value_regex = r"[^/]+" + + permission_classes = [ReadOnlyPermission | (IsAuthenticated & IsProjectAdmin)] + + def _get_filetreediff_response(self, *, request, project, version, resolver): + """ + Get the file tree diff response for the given version. + + This response is only enabled for external versions, + we do the comparison between the current version and the latest version. + """ + if not version.is_external: + return None + + if not project.addons.filetreediff_enabled: + return None + + base_version = ( + project.addons.options_base_version or project.get_latest_version() + ) + # TODO: check if `self._has_permission` is important after the migration here. + # if not base_version or not self._has_permission( + # request=request, version=base_version + # ): + if not base_version: + return None + + diff = get_diff(version_a=version, version_b=base_version) + if not diff: + return None + + return { + "outdated": diff.outdated, + "diff": { + "added": [ + { + "filename": filename, + "urls": { + "current": resolver.resolve_version( + project=project, + filename=filename, + version=version, + ), + "base": resolver.resolve_version( + project=project, + filename=filename, + version=base_version, + ), + }, + } + for filename in diff.added + ], + "deleted": [ + { + "filename": filename, + "urls": { + "current": resolver.resolve_version( + project=project, + filename=filename, + version=version, + ), + "base": resolver.resolve_version( + project=project, + filename=filename, + version=base_version, + ), + }, + } + for filename in diff.deleted + ], + "modified": [ + { + "filename": filename, + "urls": { + "current": resolver.resolve_version( + project=project, + filename=filename, + version=version, + ), + "base": resolver.resolve_version( + project=project, + filename=filename, + version=base_version, + ), + }, + } + for filename in diff.modified + ], + }, + } + + def list(self, request, **kwargs): + project = self._get_parent_project() + version = self._get_parent_version() + resolver = Resolver() + + data = ( + self._get_filetreediff_response( + request=request, + project=project, + version=version, + resolver=resolver, + ) + or {} + ) + return Response(data=data) + + class BuildsViewSet( APIv3Settings, NestedViewSetMixin, diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index 933f359fbec..74a94543b5d 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -23,7 +23,6 @@ from readthedocs.core.resolver import Resolver from readthedocs.core.unresolver import UnresolverError, unresolver from readthedocs.core.utils.extend import SettingsOverrideObject -from readthedocs.filetreediff import get_diff from readthedocs.projects.constants import ( ADDONS_FLYOUT_SORTING_CALVER, ADDONS_FLYOUT_SORTING_CUSTOM_PATTERN, @@ -406,36 +405,6 @@ def _v1(self, project, version, build, filename, url, request): data = { "api_version": "1", - "projects": { - "current": ProjectSerializerNoLinks( - project, - resolver=resolver, - version_slug=version.slug if version else None, - ).data, - "translations": ProjectSerializerNoLinks( - project_translations, - resolver=resolver, - version_slug=version.slug if version else None, - many=True, - ).data, - }, - "versions": { - "current": VersionSerializerNoLinks( - version, - resolver=resolver, - ).data - if version - else None, - # These are "sorted active, built, not hidden versions" - "active": VersionSerializerNoLinks( - sorted_versions_active_built_not_hidden, - resolver=resolver, - many=True, - ).data, - }, - "builds": { - "current": BuildSerializerNoLinks(build).data if build else None, - }, # TODO: consider creating one serializer per field here. # The resulting JSON will be the same, but maybe it's easier/cleaner? "domains": { @@ -529,21 +498,12 @@ def _v1(self, project, version, build, filename, url, request): }, }, "filetreediff": { - "enabled": False, + "enabled": project.addons.filetreediff_enabled, }, }, } if version: - response = self._get_filetreediff_response( - request=request, - project=project, - version=version, - resolver=resolver, - ) - if response: - data["addons"]["filetreediff"].update(response) - # Show the subprojects filter on the parent project and subproject # TODO: Remove these queries and try to find a way to get this data # from the resolver, which has already done these queries. @@ -621,92 +581,6 @@ def _v1(self, project, version, build, filename, url, request): return data - def _get_filetreediff_response(self, *, request, project, version, resolver): - """ - Get the file tree diff response for the given version. - - This response is only enabled for external versions, - we do the comparison between the current version and the latest version. - """ - if not version.is_external: - return None - - if not project.addons.filetreediff_enabled: - return None - - base_version = ( - project.addons.options_base_version or project.get_latest_version() - ) - if not base_version or not self._has_permission( - request=request, version=base_version - ): - return None - - diff = get_diff(version_a=version, version_b=base_version) - if not diff: - return None - - return { - "enabled": True, - "outdated": diff.outdated, - "diff": { - "added": [ - { - "filename": filename, - "urls": { - "current": resolver.resolve_version( - project=project, - filename=filename, - version=version, - ), - "base": resolver.resolve_version( - project=project, - filename=filename, - version=base_version, - ), - }, - } - for filename in diff.added - ], - "deleted": [ - { - "filename": filename, - "urls": { - "current": resolver.resolve_version( - project=project, - filename=filename, - version=version, - ), - "base": resolver.resolve_version( - project=project, - filename=filename, - version=base_version, - ), - }, - } - for filename in diff.deleted - ], - "modified": [ - { - "filename": filename, - "urls": { - "current": resolver.resolve_version( - project=project, - filename=filename, - version=version, - ), - "base": resolver.resolve_version( - project=project, - filename=filename, - version=base_version, - ), - }, - } - for filename in diff.modified - ], - }, - } - def _v2(self, project, version, build, filename, url, user): return { "api_version": "2", From cd5b0e6ac8197e1c93cd7e887f7c48993b416da2 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 16 Dec 2024 15:40:12 +0100 Subject: [PATCH 3/4] Return API URLs in the `/_/addons/` endpoint --- readthedocs/proxito/views/hosting.py | 69 ++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index 74a94543b5d..f959d78b5e1 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -1,4 +1,5 @@ """Views for hosting features.""" +import urllib.parse from functools import lru_cache import packaging @@ -7,6 +8,7 @@ from django.contrib.auth.models import AnonymousUser from django.http import Http404, JsonResponse from django.shortcuts import get_object_or_404 +from django.urls import reverse from rest_framework import permissions from rest_framework.renderers import JSONRenderer from rest_framework.views import APIView @@ -503,6 +505,73 @@ def _v1(self, project, version, build, filename, url, request): }, } + if project and version and build: + base_version_slug = ( + project.addons.options_base_version.slug + if project.addons.options_base_version + else LATEST + ) + data["readthedocs"] = { + "urls": { + "api": { + "v3": { + "projects": { + "current": reverse( + "projects-detail", + kwargs={ + "project_slug": project.slug, + }, + ), + "translations": reverse( + "projects-translations-list", + kwargs={ + "parent_lookup_main_language_project__slug": project.slug, + }, + ), + }, + "versions": { + "current": reverse( + "projects-versions-detail", + kwargs={ + "parent_lookup_project__slug": project.slug, + "version_slug": version.slug, + }, + ), + "active": reverse( + "projects-versions-list", + kwargs={ + "parent_lookup_project__slug": project.slug, + }, + ) + + "?" + + urllib.parse.urlencode({"active": True}), + }, + "builds": { + "current": reverse( + "projects-builds-detail", + kwargs={ + "parent_lookup_project__slug": project.slug, + "build_pk": build.pk, + }, + ), + }, + # project.addons.options_base_version.slug + "filetreediff": reverse( + "projects-versions-filetreediff-list", + kwargs={ + "parent_lookup_project__slug": project.slug, + "parent_lookup_version__slug": version.slug, + }, + ) + + "?" + + urllib.parse.urlencode( + {"base-version": base_version_slug} + ), + }, + }, + } + } + if version: # Show the subprojects filter on the parent project and subproject # TODO: Remove these queries and try to find a way to get this data From 3c582ba1de9b0e8aa14b0f4e3de82be6e3429c6d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 16 Dec 2024 15:52:19 +0100 Subject: [PATCH 4/4] Remove `NoLinks` serializers --- readthedocs/api/v3/serializers.py | 8 ++--- readthedocs/proxito/views/hosting.py | 53 ---------------------------- 2 files changed, 2 insertions(+), 59 deletions(-) diff --git a/readthedocs/api/v3/serializers.py b/readthedocs/api/v3/serializers.py index b271cf255fd..1bcf2aae492 100644 --- a/readthedocs/api/v3/serializers.py +++ b/readthedocs/api/v3/serializers.py @@ -358,17 +358,13 @@ class Meta: "privacy_level", ] - def __init__(self, *args, resolver=None, version_serializer=None, **kwargs): + def __init__(self, *args, resolver=None, **kwargs): super().__init__(*args, **kwargs) # Use a shared resolver to reduce the amount of DB queries while # resolving version URLs. self.resolver = kwargs.pop("resolver", Resolver()) - # Allow passing a specific serializer when initializing it. - # This is required to pass ``VersionSerializerNoLinks`` from the addons API. - self.version_serializer = version_serializer or VersionSerializer - def get_downloads(self, obj): downloads = obj.get_downloads() data = {} @@ -390,7 +386,7 @@ def get_aliases(self, obj): if obj.slug == LATEST: alias_version = obj.project.get_original_latest_version() if alias_version and alias_version.active: - return [self.version_serializer(alias_version).data] + return [VersionSerializer(alias_version).data] return [] diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index f959d78b5e1..c7e95d09627 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -15,11 +15,6 @@ from readthedocs.api.mixins import CDNCacheTagsMixin from readthedocs.api.v2.permissions import IsAuthorizedToViewVersion -from readthedocs.api.v3.serializers import ( - BuildSerializer, - ProjectSerializer, - VersionSerializer, -) from readthedocs.builds.constants import BUILD_STATE_FINISHED, LATEST from readthedocs.builds.models import Build, Version from readthedocs.core.resolver import Resolver @@ -223,54 +218,6 @@ def get(self, request, *args, **kwargs): return JsonResponse(data, json_dumps_params={"indent": 4, "sort_keys": True}) -class NoLinksMixin: - - """Mixin to remove conflicting fields from serializers.""" - - FIELDS_TO_REMOVE = ("_links",) - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - - for field in self.FIELDS_TO_REMOVE: - if field in self.fields: - del self.fields[field] - - if field in self.Meta.fields: - del self.Meta.fields[self.Meta.fields.index(field)] - - -# NOTE: the following serializers are required only to remove some fields we -# can't expose yet in this API endpoint because it's running under El Proxito -# which cannot resolve URLs pointing to the APIv3 because they are not defined -# on El Proxito. -# -# See https://github.com/readthedocs/readthedocs-ops/issues/1323 -class ProjectSerializerNoLinks(NoLinksMixin, ProjectSerializer): - def __init__(self, *args, **kwargs): - resolver = kwargs.pop("resolver", Resolver()) - super().__init__( - *args, - resolver=resolver, - **kwargs, - ) - - -class VersionSerializerNoLinks(NoLinksMixin, VersionSerializer): - def __init__(self, *args, **kwargs): - resolver = kwargs.pop("resolver", Resolver()) - super().__init__( - *args, - resolver=resolver, - version_serializer=VersionSerializerNoLinks, - **kwargs, - ) - - -class BuildSerializerNoLinks(NoLinksMixin, BuildSerializer): - pass - - class AddonsResponseBase: def get( self,