Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APIv3: proxy these URLs to be served from El Proxito /_/api/v3/ #11831

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions readthedocs/api/v3/proxied_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

@ericholscher ericholscher Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know Santos had some worried about handling modifications here. This seems like a pretty large change?

Santos mentioned:

We can cache content over docs domains (assuming we only expose read-only resources, and on .org only).

And this doesn't seem to restrict any of those things? In particular, we need to remove auth from these API endpoints (

# DRF has BasicAuthentication and SessionAuthentication as default classes.
# We don't support neither in the community site.
authentication_classes = []
), which this isn't doing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I haven't jumped into read-only endpoints / auth / cache yet because I wanted to be sure I'm moving in the right direction here before implementing those. If this POC is 👍🏼 -- we can start exploring how to achieve those goals as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I understand we want auth here because .com will require authentication requests to return the correct data for those private projects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I understand we want auth here because .com will require authentication requests to return the correct data for those private projects.

Yea, looks like we're setting SessionAuthentication explicitly on the Corporate side for the proxied APIs.

8 changes: 2 additions & 6 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand All @@ -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 []


Expand Down
12 changes: 12 additions & 0 deletions readthedocs/api/v3/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
BuildsCreateViewSet,
BuildsViewSet,
EnvironmentVariablesViewSet,
FileTreeDiffViewSet,
NotificationsBuildViewSet,
NotificationsForUserViewSet,
NotificationsOrganizationViewSet,
Expand Down Expand Up @@ -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(
Expand Down
125 changes: 123 additions & 2 deletions readthedocs/api/v3/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@
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

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,
Expand Down Expand Up @@ -110,7 +111,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

Expand Down Expand Up @@ -382,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,
Expand Down
Loading