Browse Source

ref(hc): Only set the organizationmembermapping.member_id for monolith (#49789)

Support lookup in invite with orgmember_id only for regions that were
historically monolith.

Simplify all existing organizationmembermapping logic.
Zach Collins 1 year ago
parent
commit
75833f69ce

+ 51 - 12
src/sentry/api/endpoints/accept_organization_invite.py

@@ -1,5 +1,8 @@
-from typing import Optional
+from __future__ import annotations
 
+from typing import Mapping, Optional
+
+from django.conf import settings
 from django.urls import reverse
 from rest_framework import status
 from rest_framework.request import Request
@@ -11,12 +14,19 @@ from sentry.api.invite_helper import (
     add_invite_details_to_session,
     remove_invite_details_from_session,
 )
-from sentry.models import AuthProvider, OrganizationMemberMapping
+from sentry.models import (
+    AuthProvider,
+    OrganizationMapping,
+    OrganizationMember,
+    OrganizationMemberMapping,
+)
 from sentry.services.hybrid_cloud.organization import (
     RpcUserInviteContext,
     RpcUserOrganizationContext,
     organization_service,
 )
+from sentry.silo import SiloMode
+from sentry.types.region import RegionResolutionError, get_region_by_name
 from sentry.utils import auth
 
 
@@ -41,16 +51,45 @@ class AcceptOrganizationInvite(Endpoint):
         user_id: int,
     ) -> Optional[RpcUserInviteContext]:
         if organization_slug is None:
-            member_mapping = OrganizationMemberMapping.objects.filter(
-                organizationmember_id=member_id
-            ).first()
-            if member_mapping is None:
-                return None
-            invite_context = organization_service.get_invite_by_id(
-                organization_id=member_mapping.organization_id,
-                organization_member_id=member_id,
-                user_id=user_id,
-            )
+            # TODO: Remove once the organization mapping migration is complete
+            if SiloMode.get_current_mode() == SiloMode.MONOLITH:
+                om = OrganizationMember.objects.filter(id=member_id).first()
+                if om is None:
+                    return None
+                invite_context = organization_service.get_invite_by_id(
+                    organization_id=om.organization_id,
+                    organization_member_id=member_id,
+                    user_id=user_id,
+                )
+            else:
+                member_mapping: OrganizationMemberMapping | None = None
+                member_mappings: Mapping[int, OrganizationMemberMapping] = {
+                    omm.organization_id: omm
+                    for omm in OrganizationMemberMapping.objects.filter(
+                        organizationmember_id=member_id
+                    ).all()
+                }
+                org_mappings = OrganizationMapping.objects.filter(
+                    organization_id__in=list(member_mappings.keys())
+                )
+                for mapping in org_mappings:
+                    try:
+                        if (
+                            get_region_by_name(mapping.region_name).name
+                            == settings.SENTRY_MONOLITH_REGION
+                        ):
+                            member_mapping = member_mappings.get(mapping.organization_id)
+                            break
+                    except RegionResolutionError:
+                        pass
+
+                if member_mapping is None:
+                    return None
+                invite_context = organization_service.get_invite_by_id(
+                    organization_id=member_mapping.organization_id,
+                    organization_member_id=member_id,
+                    user_id=user_id,
+                )
         else:
             invite_context = organization_service.get_invite_by_slug(
                 organization_member_id=member_id,

+ 2 - 7
src/sentry/api/endpoints/organization_member/details.py

@@ -181,13 +181,10 @@ class OrganizationMemberDetailsEndpoint(OrganizationMemberEndpoint):
 
                 if result.get("regenerate"):
                     if request.access.has_scope("member:admin"):
-                        region_outbox = None
                         with transaction.atomic():
                             member.regenerate_token()
                             member.save()
-                            region_outbox = member.save_outbox_for_update()
-                        if region_outbox:
-                            region_outbox.drain_shard(max_updates_to_drain=10)
+                        member.outbox_for_update().drain_shard(max_updates_to_drain=10)
                     else:
                         return Response({"detail": ERR_INSUFFICIENT_SCOPE}, status=400)
                 if member.token_expired:
@@ -284,7 +281,6 @@ class OrganizationMemberDetailsEndpoint(OrganizationMemberEndpoint):
             r.id for r in team_roles.get_all() if r.priority <= new_minimum_team_role.priority
         ]
 
-        region_outbox = None
         with transaction.atomic():
             # If the member has any existing team roles that are less than or equal
             # to their new minimum role, overwrite the redundant team roles with
@@ -296,8 +292,7 @@ class OrganizationMemberDetailsEndpoint(OrganizationMemberEndpoint):
             ).update(role=None)
 
             member.role = role
-            region_outbox = member.save()
-            region_outbox.drain_shard(max_updates_to_drain=10)
+            member.save()
         if omt_update_count > 0:
             metrics.incr(
                 "team_roles.update_to_minimum",

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

@@ -264,7 +264,6 @@ class OrganizationMemberIndexEndpoint(OrganizationEndpoint):
             )
             return Response({"detail": ERR_RATE_LIMITED}, status=429)
 
-        region_outbox = None
         with transaction.atomic():
             # remove any invitation requests for this email before inviting
             existing_invite = OrganizationMember.objects.filter(
@@ -286,9 +285,7 @@ class OrganizationMemberIndexEndpoint(OrganizationEndpoint):
             if settings.SENTRY_ENABLE_INVITES:
                 om.token = om.generate_token()
             om.save()
-            region_outbox = om.save_outbox_for_create()
-        if region_outbox:
-            region_outbox.drain_shard(max_updates_to_drain=10)
+        om.outbox_for_update().drain_shard(max_updates_to_drain=10)
 
         # Do not set team-roles when inviting members
         if "teamRoles" in result or "teams" in result:

+ 3 - 5
src/sentry/api/endpoints/organization_member/requests/invite/details.py

@@ -104,15 +104,13 @@ class OrganizationInviteRequestDetailsEndpoint(OrganizationMemberEndpoint):
 
         result = serializer.validated_data
 
-        region_outbox = None
         if result.get("orgRole"):
             member.role = result["orgRole"]
-            region_outbox = member.save()
+            member.save()
         elif result.get("role"):
             member.role = result["role"]
-            region_outbox = member.save()
-        if region_outbox:
-            region_outbox.drain_shard(max_updates_to_drain=10)
+            member.save()
+        member.outbox_for_update().drain_shard(max_updates_to_drain=10)
 
         # Do not set team-roles when inviting members
         if "teamRoles" in result or "teams" in result:

+ 4 - 25
src/sentry/models/organizationmember.py

@@ -251,15 +251,9 @@ class OrganizationMember(Model):
         with transaction.atomic(), in_test_psql_role_override("postgres"):
             if self.token and not self.token_expires_at:
                 self.refresh_expires_at()
-            is_new = not bool(self.id)
             super().save(*args, **kwargs)
-            region_outbox = None
-            if is_new:
-                region_outbox = self.save_outbox_for_create()
-            else:
-                region_outbox = self.save_outbox_for_update()
+            self.save_outbox_for_update()
             self.__org_roles_from_teams = None
-            return region_outbox
 
     def refresh_from_db(self, *args, **kwargs):
         super().refresh_from_db(*args, **kwargs)
@@ -280,20 +274,6 @@ class OrganizationMember(Model):
         self.token = self.generate_token()
         self.refresh_expires_at()
 
-    def outbox_for_create(self) -> RegionOutbox:
-        return RegionOutbox(
-            shard_scope=OutboxScope.ORGANIZATION_SCOPE,
-            shard_identifier=self.organization_id,
-            category=OutboxCategory.ORGANIZATION_MEMBER_CREATE,
-            object_identifier=self.id,
-            payload=dict(user_id=self.user_id),
-        )
-
-    def save_outbox_for_create(self) -> RegionOutbox:
-        outbox = self.outbox_for_create()
-        outbox.save()
-        return outbox
-
     def outbox_for_update(self) -> RegionOutbox:
         return RegionOutbox(
             shard_scope=OutboxScope.ORGANIZATION_SCOPE,
@@ -593,12 +573,9 @@ class OrganizationMember(Model):
         from sentry import audit_log
         from sentry.utils.audit import create_audit_entry_from_user
 
-        region_outbox = None
         with transaction.atomic():
             self.approve_invite()
-            region_outbox = self.save()
-        if region_outbox:
-            region_outbox.drain_shard(max_updates_to_drain=10)
+            self.save()
 
         if settings.SENTRY_ENABLE_INVITES:
             self.send_invite_email()
@@ -609,6 +586,8 @@ class OrganizationMember(Model):
                 referrer=referrer,
             )
 
+        self.outbox_for_update().drain_shard(max_updates_to_drain=10)
+
         create_audit_entry_from_user(
             user_to_approve,
             api_key,

+ 0 - 3
src/sentry/models/organizationmembermapping.py

@@ -22,9 +22,6 @@ class OrganizationMemberMapping(Model):
     __include_in_export__ = False
 
     organization_id = HybridCloudForeignKey("sentry.Organization", on_delete="CASCADE")
-    # These values are ONLY set for historical US SaaS region.  This helps bridge support for member invite tools that
-    # did not require an organization context, and only had a member_id.  However, organization member ids are not
-    # globally unique -- do not expect these to be set for other regions.
     organizationmember_id = BoundedBigIntegerField(db_index=True, null=True)
     date_added = models.DateTimeField(default=timezone.now)
 

+ 1 - 1
src/sentry/models/outbox.py

@@ -63,7 +63,7 @@ class OutboxCategory(IntEnum):
     SENTRY_APP_INSTALLATION_UPDATE = 10
     TEAM_UPDATE = 11
     ORGANIZATION_INTEGRATION_UPDATE = 12
-    ORGANIZATION_MEMBER_CREATE = 13
+    ORGANIZATION_MEMBER_CREATE = 13  # Unused
 
     @classmethod
     def as_choices(cls):

+ 8 - 10
src/sentry/receivers/outbox/region.py

@@ -59,15 +59,12 @@ def maybe_handle_joined_user(org_member: OrganizationMember) -> None:
         )
 
 
+# No longer used.
 @receiver(process_region_outbox, sender=OutboxCategory.ORGANIZATION_MEMBER_CREATE)
 def process_organization_member_create(
     object_identifier: int, payload: Any, shard_identifier: int, **kwds: Any
 ):
-    if (org_member := OrganizationMember.objects.filter(id=object_identifier).last()) is None:
-        return
-
-    organizationmember_mapping_service.create_with_organization_member(org_member=org_member)
-    maybe_handle_joined_user(org_member)
+    pass
 
 
 @receiver(process_region_outbox, sender=OutboxCategory.ORGANIZATION_MEMBER_UPDATE)
@@ -76,21 +73,22 @@ def process_organization_member_updates(
 ):
     if (org_member := OrganizationMember.objects.filter(id=object_identifier).last()) is None:
         # Delete all identities that may have been associated.  This is an implicit cascade.
-        if payload and "user_id" in payload:
+        if payload and payload.get("user_id") is not None:
             identity_service.delete_identities(
                 user_id=payload["user_id"], organization_id=shard_identifier
             )
-        organizationmember_mapping_service.delete_with_organization_member(
-            organizationmember_id=object_identifier, organization_id=shard_identifier
+        organizationmember_mapping_service.delete(
+            organizationmember_id=object_identifier,
+            organization_id=shard_identifier,
         )
         return
 
     rpc_org_member_update = RpcOrganizationMemberMappingUpdate.from_orm(org_member)
 
-    organizationmember_mapping_service.update_with_organization_member(
+    organizationmember_mapping_service.upsert_mapping(
         organizationmember_id=org_member.id,
         organization_id=shard_identifier,
-        rpc_update_org_member=rpc_org_member_update,
+        mapping=rpc_org_member_update,
     )
 
     maybe_handle_joined_user(org_member)

+ 2 - 5
src/sentry/scim/endpoints/members.py

@@ -565,12 +565,11 @@ class OrganizationSCIMMemberIndex(SCIMEndpoint):
                     organization=organization, email=result["email"], role=result["role"]
                 )
 
-                region_outbox = None
                 if member_query.exists():
                     member = member_query.first()
                     if member.token_expired:
                         member.regenerate_token()
-                        region_outbox = member.save()
+                        member.save()
                 else:
                     member = OrganizationMember(
                         organization=organization,
@@ -584,9 +583,7 @@ class OrganizationSCIMMemberIndex(SCIMEndpoint):
                     member.flags["idp:role-restricted"] = idp_role_restricted
                     if settings.SENTRY_ENABLE_INVITES:
                         member.token = member.generate_token()
-                    region_outbox = member.save()
-                if region_outbox:
-                    region_outbox.drain_shard(max_updates_to_drain=10)
+                    member.save()
 
             self.create_audit_entry(
                 request=request,

+ 3 - 13
src/sentry/services/hybrid_cloud/organization/impl.py

@@ -201,7 +201,6 @@ class DatabaseBackedOrganizationService(OrganizationService):
         organization_id: int,
         user_id: int,
     ) -> Optional[RpcOrganizationMember]:
-        region_outbox = None
         with transaction.atomic():
             try:
                 org_member = OrganizationMember.objects.get(
@@ -215,12 +214,9 @@ class DatabaseBackedOrganizationService(OrganizationService):
                     )
                     org_member.set_user(user_id)
                     org_member.save()
-                    region_outbox = org_member.outbox_for_update()
-                    region_outbox.save()
+                    org_member.outbox_for_update().drain_shard(max_updates_to_drain=10)
                 except OrganizationMember.DoesNotExist:
                     return None
-        if region_outbox:
-            region_outbox.drain_shard(max_updates_to_drain=10)
         return serialize_member(org_member)
 
     def check_organization_by_slug(self, *, slug: str, only_visible: bool) -> Optional[int]:
@@ -303,7 +299,6 @@ class DatabaseBackedOrganizationService(OrganizationService):
         ), "Must set either user_id or email"
         if invite_status is None:
             invite_status = InviteStatus.APPROVED.value
-        region_outbox = None
         with transaction.atomic(), in_test_psql_role_override("postgres"):
             org_member: OrganizationMember = OrganizationMember.objects.create(
                 organization_id=organization_id,
@@ -314,9 +309,7 @@ class DatabaseBackedOrganizationService(OrganizationService):
                 inviter_id=inviter_id,
                 invite_status=invite_status,
             )
-            region_outbox = org_member.save_outbox_for_create()
-        if region_outbox:
-            region_outbox.drain_shard(max_updates_to_drain=10)
+            org_member.outbox_for_update().drain_shard(max_updates_to_drain=10)
         return serialize_member(org_member)
 
     def add_team_member(self, *, team_id: int, organization_member: RpcOrganizationMember) -> None:
@@ -374,15 +367,12 @@ class DatabaseBackedOrganizationService(OrganizationService):
         )
 
     def remove_user(self, *, organization_id: int, user_id: int) -> RpcOrganizationMember:
-        region_outbox = None
         with transaction.atomic(), in_test_psql_role_override("postgres"):
             org_member = OrganizationMember.objects.get(
                 organization_id=organization_id, user_id=user_id
             )
             org_member.remove_user()
-            region_outbox = org_member.save()
-        if region_outbox:
-            region_outbox.drain_shard(max_updates_to_drain=10)
+            org_member.save()
         return serialize_member(org_member)
 
     def merge_users(self, *, organization_id: int, from_user_id: int, to_user_id: int) -> None:

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