Versions: enforce using project.versions instead of Version.filter(project=project) (#12411)

When using Version.filter(project=project), django will re-fetch the
project for each version when accessed, but when using
`project.versions` django will share the same project instance on each
version, so fewer queries and less memory is used. I checked and all
querysets in public make use of `self.filter`, so everything should work
properly.

There are probably more querysets that can benefit from a similar
change, but this is just a start.

Author: stsewd

readthedocs/api/v2/permissions.py

@@ -5,7 +5,6 @@
 from rest_framework_api_key.permissions import KeyParser
 
 from readthedocs.api.v2.models import BuildAPIKey
-from readthedocs.builds.models import Version
 
 
 class IsOwner(permissions.BasePermission):
@@ -39,9 +38,8 @@ def has_permission(self, request, view):
         project = view._get_project()
         version = view._get_version()
         has_access = (
-            Version.objects.public(
+            project.versions.public(
                 user=request.user,
-                project=project,
                 only_active=False,
             )
             .filter(pk=version.pk)

readthedocs/api/v2/views/core_views.py

@@ -10,7 +10,6 @@
 
 from readthedocs.api.v2.permissions import HasBuildAPIKey
 from readthedocs.builds.constants import LATEST
-from readthedocs.builds.models import Version
 from readthedocs.core.templatetags.core_tags import make_document_url
 from readthedocs.projects.models import Project
 
@@ -61,7 +60,7 @@ def docurl(request):
 
     project = get_object_or_404(Project, slug=project)
     version = get_object_or_404(
-        Version.objects.public(request.user, project=project, only_active=False),
+        project.versions.public(request.user, only_active=False),
         slug=version,
     )
     return Response(

readthedocs/builds/filters.py

@@ -7,7 +7,7 @@
 from readthedocs.builds.constants import BUILD_FINAL_STATES
 from readthedocs.builds.constants import BUILD_STATE_FINISHED
 from readthedocs.builds.constants import EXTERNAL
-from readthedocs.builds.models import Version
+from readthedocs.builds.constants import INTERNAL
 from readthedocs.core.filters import FilteredModelChoiceFilter
 from readthedocs.core.filters import ModelFilterSet
 
@@ -67,9 +67,8 @@ def get_version_queryset(self):
         # Copied from the version listing view. We need this here as this is
         # what allows the build version list to populate. Otherwise the
         # ``all()`` queryset method is used.
-        return Version.internal.public(
+        return self.project.versions(manager=INTERNAL).public(
             user=self.request.user,
-            project=self.project,
         )
 
     def get_state(self, queryset, _, value):

readthedocs/builds/querysets.py

@@ -82,7 +82,6 @@ def _public_only(self):
     def public(
         self,
         user=None,
-        project=None,
         only_active=True,
         include_hidden=True,
         only_built=False,
@@ -94,15 +93,19 @@ def public(
 
            External versions use the `Project.external_builds_privacy_level`
            field instead of its `privacy_level` field.
+
+        .. note::
+
+           Avoid filtering by reverse relationships in this method (like project),
+           and instead use project.builds or similar, so the same object is shared
+           between the results.
         """
         queryset = self._public_only()
         if user:
             if user.is_superuser:
                 queryset = self.all()
             else:
                 queryset = self._add_from_user_projects(queryset, user, admin=True, member=True)
-        if project:
-            queryset = queryset.filter(project=project)
         if only_active:
             queryset = queryset.filter(active=True)
         if only_built:

readthedocs/builds/views.py

@@ -15,9 +15,9 @@
 from requests.utils import quote
 
 from readthedocs.builds.constants import BUILD_FINAL_STATES
+from readthedocs.builds.constants import INTERNAL
 from readthedocs.builds.filters import BuildListFilter
 from readthedocs.builds.models import Build
-from readthedocs.builds.models import Version
 from readthedocs.core.filters import FilterContextMixin
 from readthedocs.core.permissions import AdminPermission
 from readthedocs.core.utils import cancel_build
@@ -55,9 +55,8 @@ class BuildList(
     filterset_class = BuildListFilter
 
     def _get_versions(self, project):
-        return Version.internal.public(
+        project.versions(manager=INTERNAL).public(
             user=self.request.user,
-            project=project,
         )
 
     def get_project(self):

readthedocs/projects/models.py

@@ -1124,9 +1124,7 @@ def latest_internal_build(self):
         return self.builds(manager=INTERNAL).select_related("version").first()
 
     def active_versions(self):
-        from readthedocs.builds.models import Version
-
-        versions = Version.internal.public(project=self, only_active=True)
+        versions = self.versions(manager=INTERNAL).public(only_active=True)
         return versions.filter(built=True, active=True) | versions.filter(
             active=True, uploaded=True
         )

readthedocs/projects/views/private.py

@@ -28,6 +28,7 @@
 from vanilla import UpdateView
 
 from readthedocs.analytics.models import PageView
+from readthedocs.builds.constants import INTERNAL
 from readthedocs.builds.forms import RegexAutomationRuleForm
 from readthedocs.builds.forms import VersionForm
 from readthedocs.builds.models import AutomationRuleMatch
@@ -234,10 +235,13 @@ def get_success_url(self):
 
 class ProjectVersionEditMixin(ProjectVersionMixin):
     def get_queryset(self):
-        return Version.internal.public(
-            user=self.request.user,
-            project=self.get_project(),
-            only_active=False,
+        return (
+            self.get_project()
+            .versions(manager=INTERNAL)
+            .public(
+                user=self.request.user,
+                only_active=False,
+            )
         )
 
     def form_valid(self, form):

readthedocs/projects/views/public.py

@@ -106,9 +106,8 @@ class ProjectDetailViewBase(
     filterset_class = ProjectVersionListFilterSet
 
     def _get_versions(self, project):
-        return Version.internal.public(
+        return project.versions(manager=INTERNAL).public(
             user=self.request.user,
-            project=project,
         )
 
     def get_queryset(self):
@@ -423,9 +422,8 @@ def project_versions(request, project_slug):
         slug=project_slug,
     )
 
-    versions = Version.internal.public(
+    versions = project.versions(manager=INTERNAL).public(
         user=request.user,
-        project=project,
         only_active=False,
     )
     active_versions = versions.filter(active=True)

readthedocs/proxito/views/hosting.py

@@ -311,10 +311,7 @@ def _get_versions(self, request, project):
         - They are active
         - They are not hidden
         """
-        # NOTE: Use project.versions, not Version.objects,
-        # so all results share the same instance of project.
         return project.versions(manager=INTERNAL).public(
-            project=project,
             user=request.user,
             only_active=True,
             only_built=True,

readthedocs/proxito/views/serve.py

@@ -14,9 +14,9 @@
 
 from readthedocs.api.mixins import CDNCacheTagsMixin
 from readthedocs.builds.constants import EXTERNAL
+from readthedocs.builds.constants import INTERNAL
 from readthedocs.builds.constants import LATEST
 from readthedocs.builds.constants import STABLE
-from readthedocs.builds.models import Version
 from readthedocs.core.mixins import CDNCacheControlMixin
 from readthedocs.core.resolver import Resolver
 from readthedocs.core.unresolver import InvalidExternalVersionError
@@ -713,7 +713,7 @@ def get(self, request):
 
     def _get_hidden_paths(self, project):
         """Get the absolute paths of the public hidden versions of `project`."""
-        hidden_versions = Version.internal.public(project=project).filter(hidden=True)
+        hidden_versions = project.versions(manager=INTERNAL).public().filter(hidden=True)
         resolver = Resolver()
         hidden_paths = [
             resolver.resolve_path(project, version_slug=version.slug) for version in hidden_versions
@@ -803,8 +803,7 @@ def changefreqs_generator():
             yield from itertools.chain(changefreqs, itertools.repeat("monthly"))
 
         project = request.unresolved_domain.project
-        public_versions = Version.internal.public(
-            project=project,
+        public_versions = project.versions(manager=INTERNAL).public(
             only_active=True,
             include_hidden=False,
         )
@@ -851,7 +850,8 @@ def changefreqs_generator():
             if project.translations.exists():
                 for translation in project.translations.all():
                     translated_version = (
-                        Version.internal.public(project=translation)
+                        translation.versions(manager=INTERNAL)
+                        .public()
                         .filter(slug=version.slug)
                         .first()
                     )