Search: optimize queryset when searching for subprojects with organizations (#12635)

On .com having organizations introduces some extra queries. Since all
subprojects share the same organization as the parent project, we can
cache that organization, so it's shared across all subprojects.

This also allows caching the .organization property, and skips querying
organizations if we are on .org.

Results:

- One less query for searches on projects with organizations
- When searching across subprojects, two queries for each subproject
(constant). It used to add 4 queries for each subproject.

I had to move tests to .com, so they are more accurate with production,
as in .com we do several overrides.

Author: stsewd

readthedocs/api/v3/tests/test_projects.py

@@ -212,7 +212,10 @@ def test_projects_detail_other_user(self):
         response = self.client.get(url, data)
         self.assertEqual(response.status_code, 404)
 
-    @override_settings(ALLOW_PRIVATE_REPOS=True)
+    @override_settings(
+        ALLOW_PRIVATE_REPOS=True,
+        RTD_ALLOW_ORGANIZATIONS=True,
+    )
     def test_own_projects_detail_privacy_levels_enabled(self):
         url = reverse(
             "projects-detail",
@@ -227,10 +230,12 @@ def test_own_projects_detail_privacy_levels_enabled(self):
         self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
         response = self.client.get(url, query_params)
         self.assertEqual(response.status_code, 200)
-        self.assertDictEqual(
-            response.json(),
-            self._get_response_dict("projects-detail"),
-        )
+
+        expected = self._get_response_dict("projects-detail")
+        # No users when organzations are enabled
+        expected.pop("users")
+
+        self.assertDictEqual(response.json(), expected)
 
         self.project.privacy_level = "private"
         self.project.external_builds_privacy_level = "private"
@@ -245,6 +250,10 @@ def test_own_projects_detail_privacy_levels_enabled(self):
         # We don't care about the modified date.
         expected.pop("modified")
         response.pop("modified")
+
+        # No users when organzations are enabled
+        expected.pop("users")
+
         self.assertDictEqual(response, expected)
 
     def test_projects_superproject_anonymous_user(self):

readthedocs/api/v3/tests/test_subprojects.py

@@ -155,7 +155,6 @@ def test_projects_subprojects_detail_other_user(self):
 
     def test_projects_subprojects_list_post(self):
         newproject = self._create_new_project()
-        self.organization.projects.add(newproject)
 
         self.assertEqual(self.project.subprojects.count(), 1)
         url = reverse(

readthedocs/audit/models.py

@@ -223,7 +223,7 @@ def save(self, **kwargs):
         if self.project:
             self.log_project_id = self.project.id
             self.log_project_slug = self.project.slug
-            organization = self.project.organizations.first()
+            organization = self.project.organization
             if organization:
                 self.organization = organization
         if self.organization:

readthedocs/audit/tests/test_models.py

@@ -1,12 +1,13 @@
 from django.contrib.auth.models import User
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from django_dynamic_fixture import get
 
 from readthedocs.audit.models import AuditLog
 from readthedocs.organizations.models import Organization, OrganizationOwner
 from readthedocs.projects.models import Project
 
 
+@override_settings(RTD_ALLOW_ORGANIZATIONS=True)
 class TestAuditModels(TestCase):
     def setUp(self):
         self.user = get(User)

readthedocs/audit/tests/test_tasks.py

@@ -1,7 +1,7 @@
 from unittest import mock
 
 from django.contrib.auth.models import User
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from django.utils import timezone
 from django_dynamic_fixture import get
 
@@ -11,6 +11,7 @@
 from readthedocs.projects.models import Project
 
 
+@override_settings(RTD_ALLOW_ORGANIZATIONS=True)
 class AuditTasksTest(TestCase):
     def setUp(self):
         self.user = get(User)

readthedocs/builds/querysets.py

@@ -246,7 +246,7 @@ def concurrent(self, project):
 
         # If the project belongs to an organization, count all the projects
         # from this organization as well
-        organization = project.organizations.first()
+        organization = project.organization
         if organization:
             query |= Q(project__in=organization.projects.all())
 

readthedocs/builds/tests/test_build_queryset.py

@@ -1,3 +1,4 @@
+from django.test import override_settings
 import django_dynamic_fixture as fixture
 import pytest
 
@@ -79,6 +80,7 @@ def test_concurrent_builds_translations(self):
         assert (True, 4, 4) == Build.objects.concurrent(translation)
         assert (True, 4, 4) == Build.objects.concurrent(project)
 
+    @override_settings(RTD_ALLOW_ORGANIZATIONS=True)
     def test_concurrent_builds_organization(self):
         organization = fixture.get(
             Organization,
@@ -111,6 +113,7 @@ def test_concurrent_builds_organization(self):
             )
         assert (True, 6, 4) == Build.objects.concurrent(project)
 
+    @override_settings(RTD_ALLOW_ORGANIZATIONS=True)
     def test_concurrent_builds_organization_limited(self):
         organization = fixture.get(
             Organization,
@@ -138,6 +141,7 @@ def test_concurrent_builds_organization_limited(self):
         # from ``project_with_builds`` as well
         assert (False, 2, 10) == Build.objects.concurrent(project_without_builds)
 
+    @override_settings(RTD_ALLOW_ORGANIZATIONS=True)
     def test_concurrent_builds_organization_and_project_limited(self):
         organization = fixture.get(
             Organization,

readthedocs/core/permissions.py

@@ -118,7 +118,7 @@ def owners(cls, obj):
 
         if isinstance(obj, Project):
             if settings.RTD_ALLOW_ORGANIZATIONS:
-                obj = obj.organizations.first()
+                obj = obj.organization
             else:
                 return obj.users.all()
 

readthedocs/core/resolver.py

@@ -198,11 +198,16 @@ def _get_project_domain(self, project, external_version_slug=None, use_canonical
         domain = self._get_project_subdomain(canonical_project)
         if external_version_slug:
             domain = self._get_external_subdomain(canonical_project, external_version_slug)
-        elif use_canonical_domain and self._use_cname(canonical_project):
+        # NOTE: call _use_cname only if the project has a canonical custom domain.
+        # Calling _use_cname is more expensive when we have organizations.
+        elif (
+            use_canonical_domain
+            and canonical_project.canonical_custom_domain
+            and self._use_cname(canonical_project)
+        ):
             domain_object = canonical_project.canonical_custom_domain
-            if domain_object:
-                use_https = domain_object.https
-                domain = domain_object.domain
+            use_https = domain_object.https
+            domain = domain_object.domain
 
         return domain, use_https
 
@@ -365,4 +370,22 @@ def _fix_filename(self, filename):
 
     def _use_cname(self, project):
         """Test if to allow direct serving for project on CNAME."""
+        # If the project belongs to an organization, use the other
+        # method, to allow caching the result for that organization.
+        if project.organization:
+            return self._organization_allows_custom_domain(project.organization)
+
         return bool(get_feature(project, feature_type=TYPE_CNAME))
+
+    @cache
+    def _organization_allows_custom_domain(self, organization):
+        """
+        Test if the organization allows custom domains.
+
+        .. note::
+
+           This is done on a separate method, so we can cache the result
+           for each organization. This is useful when resolving multiple
+           projects from the same organization.
+        """
+        return bool(get_feature(organization, feature_type=TYPE_CNAME))

readthedocs/invitations/tests/test_views.py

@@ -1,5 +1,5 @@
 from django.contrib.auth.models import User
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from django.urls import reverse
 from django.utils import timezone
 from django_dynamic_fixture import get
@@ -10,6 +10,7 @@
 from readthedocs.projects.models import Project
 
 
+@override_settings(RTD_ALLOW_ORGANIZATIONS=True)
 class TestViews(TestCase):
     def setUp(self):
         self.user = get(User)