Browse Source

ref(hc): Removing user relation from OrganizationMember usages (migration separate) (#50674)

Removing all usages of OrganizationMember.user from query code paths.
Addresses the known issues around GroupSubscriptions seen in the last
release, along with additional testing to prevent regression in future.
Zach Collins 1 year ago
parent
commit
b953b09112

+ 4 - 5
src/sentry/api/bases/organizationmember.py

@@ -2,7 +2,6 @@ from __future__ import annotations
 
 from typing import Any
 
-from django.db.models import Q
 from rest_framework import serializers
 from rest_framework.request import Request
 
@@ -67,12 +66,12 @@ class OrganizationMemberEndpoint(OrganizationEndpoint):
         kwargs = dict(organization=organization)
 
         if member_id == "me":
-            kwargs.update(user__id=request.user.id, user__is_active=True)
+            kwargs.update(user_id=request.user.id, user_is_active=True)
         else:
-            args.append(Q(user__is_active=True) | Q(user__isnull=True))
-            kwargs.update(id=member_id)
+            kwargs.update(id=member_id, organization_id=organization.id)
 
         if invite_status:
             kwargs.update(invite_status=invite_status.value)
 
-        return OrganizationMember.objects.filter(*args, **kwargs).select_related("user").get()
+        om = OrganizationMember.objects.filter(*args, **kwargs).get()
+        return om

+ 1 - 1
src/sentry/api/bases/user.py

@@ -35,7 +35,7 @@ class OrganizationUserPermission(UserPermission):
 
         try:
             organization = Organization.objects.get(
-                status=OrganizationStatus.ACTIVE, member_set__user=user
+                status=OrganizationStatus.ACTIVE, member_set__user_id=user.id
             )
 
             self.determine_access(request, organization)

+ 7 - 4
src/sentry/api/endpoints/organization_access_request_details.py

@@ -69,16 +69,19 @@ class OrganizationAccessRequestDetailsEndpoint(OrganizationEndpoint):
         if request.access.has_scope("org:write"):
             access_requests = list(
                 OrganizationAccessRequest.objects.filter(
-                    team__organization=organization, member__user__is_active=True
-                ).select_related("team", "member__user")
+                    team__organization=organization,
+                    member__user_is_active=True,
+                    member__user_id__isnull=False,
+                ).select_related("team")
             )
 
         elif request.access.has_scope("team:write") and request.access.team_ids_with_membership:
             access_requests = list(
                 OrganizationAccessRequest.objects.filter(
-                    member__user__is_active=True,
+                    member__user_is_active=True,
+                    member__user_id__isnull=False,
                     team__id__in=request.access.team_ids_with_membership,
-                ).select_related("team", "member__user")
+                ).select_related("team")
             )
         else:
             # Return empty response if user does not have access

+ 5 - 2
src/sentry/api/endpoints/organization_index.py

@@ -24,6 +24,7 @@ from sentry.models import (
 )
 from sentry.search.utils import tokenize_query
 from sentry.services.hybrid_cloud import IDEMPOTENCY_KEY_LENGTH
+from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.signals import org_setup_complete, terms_accepted
 
 
@@ -103,15 +104,17 @@ class OrganizationIndexEndpoint(Endpoint):
             for key, value in tokens.items():
                 if key == "query":
                     value = " ".join(value)
+                    user_ids = {u.id for u in user_service.get_many_by_email(emails=[value])}
                     queryset = queryset.filter(
                         Q(name__icontains=value)
                         | Q(slug__icontains=value)
-                        | Q(members__email__iexact=value)
+                        | Q(member_set__user_id__in=user_ids)
                     )
                 elif key == "slug":
                     queryset = queryset.filter(in_iexact("slug", value))
                 elif key == "email":
-                    queryset = queryset.filter(in_iexact("members__email", value))
+                    user_ids = {u.id for u in user_service.get_many_by_email(emails=value)}
+                    queryset = queryset.filter(Q(member_set__user_id__in=user_ids))
                 elif key == "platform":
                     queryset = queryset.filter(
                         project__in=ProjectPlatform.objects.filter(platform__in=value).values(

+ 11 - 10
src/sentry/api/endpoints/organization_member/details.py

@@ -26,9 +26,9 @@ from sentry.models import (
     OrganizationMember,
     OrganizationMemberTeam,
     Project,
-    UserOption,
 )
 from sentry.roles import organization_roles, team_roles
+from sentry.services.hybrid_cloud.user_option import user_option_service
 from sentry.utils import metrics
 
 from . import InvalidTeam, get_allowed_org_roles, save_team_assignments
@@ -240,7 +240,7 @@ class OrganizationMemberDetailsEndpoint(OrganizationMemberEndpoint):
                     status=403,
                 )
 
-            if member.user == request.user and (assigned_org_role != member.role):
+            if member.user_id == request.user.id and (assigned_org_role != member.role):
                 return Response({"detail": "You cannot make changes to your own role."}, status=400)
 
             if (
@@ -362,15 +362,16 @@ class OrganizationMemberDetailsEndpoint(OrganizationMemberEndpoint):
         with transaction.atomic():
             # Delete instances of `UserOption` that are scoped to the projects within the
             # organization when corresponding member is removed from org
-            proj_list = Project.objects.filter(organization=organization).values_list(
-                "id", flat=True
+            proj_list = list(
+                Project.objects.filter(organization=organization).values_list("id", flat=True)
             )
-            uo_list = UserOption.objects.filter(
-                user=member.user, project_id__in=proj_list, key="mail:email"
-            )
-            for uo in uo_list:
-                uo.delete()
-
+            uos = [
+                uo
+                for uo in user_option_service.get_many(
+                    filter=dict(user_ids=[member.user_id], project_ids=proj_list, key="mail:email")
+                )
+            ]
+            user_option_service.delete_options(option_ids=[uo.id for uo in uos])
             member.delete()
 
         self.create_audit_entry(

+ 26 - 24
src/sentry/api/endpoints/organization_member/index.py

@@ -19,6 +19,7 @@ from sentry.models import ExternalActor, InviteStatus, OrganizationMember, Team,
 from sentry.models.authenticator import available_authenticators
 from sentry.roles import organization_roles, team_roles
 from sentry.search.utils import tokenize_query
+from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.signals import member_invited
 from sentry.utils import metrics
 
@@ -55,8 +56,11 @@ class OrganizationMemberSerializer(serializers.Serializer):
     regenerate = serializers.BooleanField(required=False)
 
     def validate_email(self, email):
+        users = user_service.get_many_by_email(
+            emails=[email], is_active=True, organization_id=self.context["organization"].id
+        )
         queryset = OrganizationMember.objects.filter(
-            Q(email=email) | Q(user__email__iexact=email, user__is_active=True),
+            Q(email=email) | Q(user_id__in=[u.id for u in users]),
             organization=self.context["organization"],
         )
 
@@ -116,32 +120,29 @@ class OrganizationMemberIndexEndpoint(OrganizationEndpoint):
     permission_classes = (MemberPermission,)
 
     def get(self, request: Request, organization) -> Response:
-        queryset = (
-            OrganizationMember.objects.filter(
-                Q(user__is_active=True) | Q(user__isnull=True),
-                organization=organization,
-                invite_status=InviteStatus.APPROVED.value,
-            )
-            # TODO(hybridcloud) Cross silo joins here.
-            .select_related("user").order_by("email", "user__email")
-        )
+        queryset = OrganizationMember.objects.filter(
+            Q(user_is_active=True, user_id__isnull=False) | Q(user_id__isnull=True),
+            organization=organization,
+            invite_status=InviteStatus.APPROVED.value,
+        ).order_by("id")
 
         query = request.GET.get("query")
         if query:
             tokens = tokenize_query(query)
             for key, value in tokens.items():
                 if key == "email":
+                    email_user_ids = user_service.get_many_by_email(
+                        emails=value, organization_id=organization.id
+                    )
                     queryset = queryset.filter(
-                        Q(email__in=value)
-                        | Q(user__email__in=value)
-                        | Q(user__emails__email__in=value)
+                        Q(email__in=value) | Q(user_id__in=[u.id for u in email_user_ids])
                     )
 
                 elif key == "id":
                     queryset = queryset.filter(id__in=value)
 
                 elif key == "user.id":
-                    queryset = queryset.filter(user__id__in=value)
+                    queryset = queryset.filter(user_id__in=value)
 
                 elif key == "scope":
                     queryset = queryset.filter(role__in=[r.id for r in roles.with_any_scope(value)])
@@ -154,7 +155,7 @@ class OrganizationMemberIndexEndpoint(OrganizationEndpoint):
 
                 elif key == "isInvited":
                     isInvited = "true" in value
-                    queryset = queryset.filter(user__isnull=isInvited)
+                    queryset = queryset.filter(user_id__isnull=isInvited)
 
                 elif key == "ssoLinked":
                     ssoFlag = OrganizationMember.flags["sso:linked"]
@@ -165,15 +166,18 @@ class OrganizationMemberIndexEndpoint(OrganizationEndpoint):
                         queryset = queryset.filter(flags=F("flags").bitand(~ssoFlag))
 
                 elif key == "has2fa":
-                    # TODO(hybridcloud) Cross silo joins here.
                     has2fa = "true" in value
                     if has2fa:
                         types = [a.type for a in available_authenticators(ignore_backup=True)]
-                        queryset = queryset.filter(
-                            user__authenticator__isnull=False, user__authenticator__type__in=types
-                        ).distinct()
+                        has2fa_user_ids = user_service.get_many_ids(
+                            filter=dict(organization_id=organization.id, authenticator_types=types)
+                        )
+                        queryset = queryset.filter(user_id__in=has2fa_user_ids).distinct()
                     else:
-                        queryset = queryset.filter(user__authenticator__isnull=True)
+                        has2fa_user_ids = user_service.get_many_ids(
+                            filter=dict(organization_id=organization.id, authenticator_types=None)
+                        )
+                        queryset = queryset.filter(user_id__in=has2fa_user_ids).distinct()
                 elif key == "hasExternalUsers":
                     externalactor_user_ids = ExternalActor.objects.filter(
                         organization=organization,
@@ -186,11 +190,9 @@ class OrganizationMemberIndexEndpoint(OrganizationEndpoint):
                         queryset = queryset.exclude(user_id__in=externalactor_user_ids)
                 elif key == "query":
                     value = " ".join(value)
-                    # TODO(hybridcloud) Cross silo joins.
+                    query_user_ids = user_service.get_many_ids(filter=dict(query=value))
                     queryset = queryset.filter(
-                        Q(email__icontains=value)
-                        | Q(user__email__icontains=value)
-                        | Q(user__name__icontains=value)
+                        Q(user_id__in=query_user_ids) | Q(email__icontains=value)
                     )
                 else:
                     queryset = queryset.none()

+ 1 - 1
src/sentry/api/endpoints/organization_member/requests/invite/index.py

@@ -30,7 +30,7 @@ class OrganizationInviteRequestIndexEndpoint(OrganizationEndpoint):
 
     def get(self, request: Request, organization) -> Response:
         queryset = OrganizationMember.objects.filter(
-            Q(user__isnull=True),
+            Q(user_id__isnull=True),
             Q(invite_status=InviteStatus.REQUESTED_TO_BE_INVITED.value)
             | Q(invite_status=InviteStatus.REQUESTED_TO_JOIN.value),
             organization=organization,

+ 2 - 1
src/sentry/api/endpoints/organization_member/requests/join.py

@@ -26,7 +26,8 @@ class JoinRequestSerializer(serializers.Serializer):
 
 def create_organization_join_request(organization, email, ip_address=None):
     om = OrganizationMember.objects.filter(
-        Q(email__iexact=email) | Q(user__is_active=True, user__email__iexact=email),
+        Q(email__iexact=email)
+        | Q(user_is_active=True, user_email__iexact=email, user_id__isnull=False),
         organization=organization,
     ).first()
     if om:

+ 10 - 13
src/sentry/api/endpoints/organization_users.py

@@ -26,19 +26,16 @@ class OrganizationUsersEndpoint(OrganizationEndpoint, EnvironmentMixin):
         projects = self.get_projects(request, organization)
 
         with sentry_sdk.start_span(op="OrganizationUsersEndpoint.get_members") as span:
-            qs = (
-                OrganizationMember.objects.filter(
-                    user__is_active=True,
-                    organization=organization,
-                    id__in=OrganizationMemberTeam.objects.filter(
-                        team_id__in=ProjectTeam.objects.filter(project_id__in=projects)
-                        .values_list("team_id", flat=True)
-                        .distinct(),
-                    ).values_list("organizationmember_id", flat=True),
-                )
-                .select_related("user")
-                .order_by("user_email")
-            )
+            qs = OrganizationMember.objects.filter(
+                user_id__isnull=False,
+                user_is_active=True,
+                organization=organization,
+                id__in=OrganizationMemberTeam.objects.filter(
+                    team_id__in=ProjectTeam.objects.filter(project_id__in=projects)
+                    .values_list("team_id", flat=True)
+                    .distinct(),
+                ).values_list("organizationmember_id", flat=True),
+            ).order_by("user_email")
 
             organization_members = list(qs)
 

+ 1 - 1
src/sentry/api/endpoints/project_index.py

@@ -53,7 +53,7 @@ class ProjectIndexEndpoint(Endpoint):
                 if isinstance(queryset, EmptyQuerySet):
                     raise AuthenticationFailed("Token not found")
             else:
-                queryset = queryset.filter(teams__organizationmember__user=request.user)
+                queryset = queryset.filter(teams__organizationmember__user_id=request.user.id)
 
         query = request.GET.get("query")
         if query:

Some files were not shown because too many files changed in this diff