Browse Source

fix(hybridcloud) Fix an N+1 query in codeowners and cross silo joins (#48939)

We're doing an N+1 query for actors here, and we're also doing a
cross-silo join from UserEmail to Organization.
Mark Story 1 year ago
parent
commit
54ebfc0c21

+ 14 - 8
src/sentry/api/validators/project_codeowners.py

@@ -13,6 +13,7 @@ from sentry.models import (
     actor_type_to_string,
 )
 from sentry.ownership.grammar import parse_code_owners
+from sentry.services.hybrid_cloud.user import user_service
 from sentry.types.integrations import ExternalProviders
 
 if TYPE_CHECKING:
@@ -26,8 +27,8 @@ def validate_association(
 ) -> Sequence[str]:
     raw_items_set = {str(item) for item in raw_items}
     if type == "emails":
-        # associations are UserEmail objects
-        sentry_items = {item.email for item in associations}
+        # associations email strings
+        sentry_items = associations
     else:
         # associations are ExternalActor objects
         sentry_items = {item.external_name for item in associations}
@@ -41,17 +42,16 @@ def validate_codeowners_associations(
     team_names, usernames, emails = parse_code_owners(codeowners)
 
     # Check if there exists Sentry users with the emails listed in CODEOWNERS
-    user_emails = UserEmail.objects.filter(
-        email__in=emails,
-        user__sentry_orgmember_set__organization=project.organization,
+    users = user_service.get_many(
+        filter=dict(emails=emails, organization_id=project.organization_id)
     )
 
     # Check if the usernames/teamnames have an association
     external_actors = ExternalActor.objects.filter(
         external_name__in=usernames + team_names,
-        organization=project.organization,
+        organization_id=project.organization_id,
         provider__in=[ExternalProviders.GITHUB.value, ExternalProviders.GITLAB.value],
-    )
+    ).select_related("actor")
 
     # Convert CODEOWNERS into IssueOwner syntax
     users_dict = {}
@@ -83,7 +83,13 @@ def validate_codeowners_associations(
             else:
                 teams_without_access.append(f"#{team.slug}")
 
-    emails_dict = {item.email: item.email for item in user_emails}
+    emails_dict = {}
+    user_emails = set()
+    for user in users:
+        for user_email in user.emails:
+            emails_dict[user_email] = user_email
+            user_emails.add(user_email)
+
     associations = {**users_dict, **teams_dict, **emails_dict}
 
     errors = {

+ 1 - 1
tests/sentry/api/endpoints/test_project_codeowners.py

@@ -7,7 +7,7 @@ from sentry.testutils import APITestCase
 from sentry.testutils.silo import region_silo_test
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class ProjectCodeOwnersEndpointTestCase(APITestCase):
     def setUp(self):
         self.user = self.create_user("admin@sentry.io", is_superuser=True)

+ 1 - 1
tests/sentry/api/endpoints/test_project_codeowners_details.py

@@ -9,7 +9,7 @@ from sentry.testutils import APITestCase
 from sentry.testutils.silo import region_silo_test
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class ProjectCodeOwnersDetailsEndpointTestCase(APITestCase):
     def setUp(self):
         self.user = self.create_user("admin@sentry.io", is_superuser=True)