Browse Source

ref(hybrid-cloud): Adds new organization provisioning service, splits slug logic into separate service call (#57089)

Gabe Villalobos 1 year ago
parent
commit
3dade5d409

+ 23 - 13
src/sentry/api/endpoints/organization_details.py

@@ -4,7 +4,7 @@ import logging
 from copy import copy
 from datetime import datetime, timedelta, timezone
 
-from django.db import IntegrityError, models, router, transaction
+from django.db import models, router, transaction
 from django.db.models.query_utils import DeferredAttribute
 from django.urls import reverse
 from django.utils import timezone as django_timezone
@@ -39,7 +39,6 @@ from sentry.models import (
     OrganizationAvatar,
     OrganizationOption,
     OrganizationStatus,
-    OutboxFlushError,
     RegionScheduledDeletion,
     UserEmail,
 )
@@ -52,6 +51,10 @@ from sentry.services.hybrid_cloud.organization.model import (
     RpcOrganizationDeleteState,
 )
 from sentry.services.hybrid_cloud.user.serial import serialize_generic_user
+from sentry.services.organization.provisioning import (
+    OrganizationSlugCollisionException,
+    organization_provisioning_service,
+)
 from sentry.utils.audit import create_audit_entry
 from sentry.utils.cache import memoize
 
@@ -585,17 +588,24 @@ class OrganizationDetailsEndpoint(OrganizationEndpoint):
             context={"organization": organization, "user": request.user, "request": request},
         )
         if serializer.is_valid():
-            changed_data = {}
-            try:
-                with transaction.atomic(router.db_for_write(Organization)):
-                    organization, changed_data = serializer.save()
-            except IntegrityError:
-                return self.respond(
-                    {"slug": ["An organization with this slug already exists."]},
-                    status=status.HTTP_409_CONFLICT,
-                )
-            except OutboxFlushError:
-                pass
+            with transaction.atomic(router.db_for_write(Organization)):
+                slug_change_requested = "slug" in request.data and request.data["slug"]
+
+                # Start with the slug change first, as this may fail independent of
+                # the remaining organization changes.
+                if slug_change_requested:
+                    slug = request.data["slug"]
+                    try:
+                        organization_provisioning_service.modify_organization_slug(
+                            organization_id=organization.id, slug=slug
+                        )
+                    except OrganizationSlugCollisionException:
+                        return self.respond(
+                            {"slug": ["An organization with this slug already exists."]},
+                            status=status.HTTP_409_CONFLICT,
+                        )
+
+                organization, changed_data = serializer.save()
 
             if was_pending_deletion:
                 self.create_audit_entry(

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

@@ -18,13 +18,13 @@ from sentry.db.models.query import in_iexact
 from sentry.models import Organization, OrganizationMember, OrganizationStatus, ProjectPlatform
 from sentry.search.utils import tokenize_query
 from sentry.services.hybrid_cloud import IDEMPOTENCY_KEY_LENGTH
-from sentry.services.hybrid_cloud.organization_provisioning import (
+from sentry.services.hybrid_cloud.user.service import user_service
+from sentry.services.organization import (
     OrganizationOptions,
     OrganizationProvisioningOptions,
     PostProvisionOptions,
-    organization_provisioning_service,
 )
-from sentry.services.hybrid_cloud.user.service import user_service
+from sentry.services.organization.provisioning import organization_provisioning_service
 from sentry.signals import org_setup_complete, terms_accepted
 
 
@@ -229,9 +229,9 @@ class OrganizationIndexEndpoint(Endpoint):
                     ),
                 )
 
-                rpc_org = organization_provisioning_service.provision_organization(
+                rpc_org = organization_provisioning_service.provision_organization_in_region(
                     region_name=settings.SENTRY_MONOLITH_REGION,
-                    org_provision_args=provision_args,
+                    provisioning_options=provision_args,
                 )
                 org = Organization.objects.get(id=rpc_org.id)
 

+ 2 - 4
src/sentry/services/hybrid_cloud/organization_provisioning/impl.py

@@ -17,10 +17,8 @@ from sentry.services.hybrid_cloud.organization.serial import serialize_rpc_organ
 from sentry.services.hybrid_cloud.organization_actions.impl import (
     create_organization_and_member_for_monolith,
 )
-from sentry.services.hybrid_cloud.organization_provisioning import (
-    OrganizationProvisioningOptions,
-    OrganizationProvisioningService,
-)
+from sentry.services.hybrid_cloud.organization_provisioning import OrganizationProvisioningService
+from sentry.services.organization import OrganizationProvisioningOptions
 
 
 class SlugMismatchException(Exception):

+ 6 - 19
src/sentry/services/hybrid_cloud/organization_provisioning/model.py

@@ -1,24 +1,11 @@
-from typing import Any, Union
-
 import pydantic
 
-
-class OrganizationOptions(pydantic.BaseModel):
-    name: str
-    slug: str
-    owning_user_id: int
-    create_default_team: bool = True
-    is_test = False
-
-
-class PostProvisionOptions(pydantic.BaseModel):
-    sentry_options: Union[Any, None]  # Placeholder for any sentry post-provisioning data
-    getsentry_options: Union[Any, None]  # Reserved for getsentry post-provisioning data
-
-
-class OrganizationProvisioningOptions(pydantic.BaseModel):
-    provision_options: OrganizationOptions
-    post_provision_options: PostProvisionOptions
+# TODO(Gabe): Remove this once GetSentry has been updated to use new model file in org provisioning
+from sentry.services.organization.model import (  # noqa
+    OrganizationOptions,
+    OrganizationProvisioningOptions,
+    PostProvisionOptions,
+)
 
 
 class RpcOrganizationSlugReservation(pydantic.BaseModel):

+ 1 - 1
src/sentry/services/hybrid_cloud/organization_provisioning/service.py

@@ -6,9 +6,9 @@ from abc import abstractmethod
 from typing import Optional, cast
 
 from sentry.services.hybrid_cloud.organization import RpcOrganization
-from sentry.services.hybrid_cloud.organization_provisioning import OrganizationProvisioningOptions
 from sentry.services.hybrid_cloud.region import ByRegionName
 from sentry.services.hybrid_cloud.rpc import RpcService, regional_rpc_method
+from sentry.services.organization.model import OrganizationProvisioningOptions
 from sentry.silo import SiloMode
 
 

+ 2 - 0
src/sentry/services/organization/__init__.py

@@ -0,0 +1,2 @@
+from .model import *  # noqa
+from .provisioning import *  # noqa

+ 21 - 0
src/sentry/services/organization/model.py

@@ -0,0 +1,21 @@
+from typing import Any, Union
+
+import pydantic
+
+
+class OrganizationOptions(pydantic.BaseModel):
+    name: str
+    slug: str
+    owning_user_id: int
+    create_default_team: bool = True
+    is_test = False
+
+
+class PostProvisionOptions(pydantic.BaseModel):
+    sentry_options: Union[Any, None]  # Placeholder for any sentry post-provisioning data
+    getsentry_options: Union[Any, None]  # Reserved for getsentry post-provisioning data
+
+
+class OrganizationProvisioningOptions(pydantic.BaseModel):
+    provision_options: OrganizationOptions
+    post_provision_options: PostProvisionOptions

+ 70 - 0
src/sentry/services/organization/provisioning.py

@@ -0,0 +1,70 @@
+from typing import Optional
+
+from django.conf import settings
+from django.db import IntegrityError, router, transaction
+
+from sentry.services.organization.model import OrganizationProvisioningOptions
+
+
+class OrganizationSlugCollisionException(Exception):
+    pass
+
+
+class OrganizationProvisioningService:
+    def provision_organization_in_region(
+        self, provisioning_options: OrganizationProvisioningOptions, region_name: Optional[str]
+    ):
+        """
+        Provisions an organization in the provided region. If no region is
+        provided, the default monolith region is assumed.
+
+        This method is fairly slim at the moment, solely because it's acting
+        as a proxy for the underlying RPC service. There will be more
+        provisioning logic added when this is made multi-region safe.
+
+        :param provisioning_options: The organization provisioning and post-
+        provisioning options
+        :param region_name: The region name to provision the organization in.
+        :return: RPCOrganization
+        """
+        if region_name is None:
+            region_name = settings.SENTRY_MONOLITH_REGION
+
+        from sentry.services.hybrid_cloud.organization_provisioning import (
+            organization_provisioning_service as rpc_org_provisioning_service,
+        )
+
+        rpc_org = rpc_org_provisioning_service.provision_organization(
+            region_name=region_name, org_provision_args=provisioning_options
+        )
+
+        return rpc_org
+
+    def idempotent_provision_organization_in_region(
+        self, provisioning_options: OrganizationProvisioningOptions, region_name: Optional[str]
+    ):
+        raise NotImplementedError()
+
+    def modify_organization_slug(self, organization_id: int, slug: str):
+        """
+        Updates an organization with the given slug if available.
+
+         This is currently database backed, but will be switched to be
+         RPC based in the near future.
+        :param organization_id:
+        :param slug:
+        :return:
+        """
+
+        from sentry.models import Organization
+
+        try:
+            with transaction.atomic(using=router.db_for_write(Organization)):
+                organization = Organization.objects.get(id=organization_id)
+                organization.slug = slug
+                organization.save()
+        except IntegrityError:
+            raise OrganizationSlugCollisionException()
+
+
+organization_provisioning_service = OrganizationProvisioningService()

+ 0 - 44
tests/sentry/api/endpoints/test_organization_details.py

@@ -29,10 +29,8 @@ from sentry.models import (
     OrganizationAvatar,
     OrganizationOption,
     OrganizationStatus,
-    OutboxFlushError,
     RegionScheduledDeletion,
     User,
-    outbox_context,
 )
 from sentry.models.organizationmapping import OrganizationMapping
 from sentry.signals import project_created
@@ -855,48 +853,6 @@ class OrganizationUpdateTest(OrganizationDetailsTestBase):
         organization_mapping.refresh_from_db()
         assert organization_mapping.slug == desired_slug
 
-    def test_update_slug_with_temporary_rename_collision(self):
-        desired_slug = "taken"
-        previous_slug = self.organization.slug
-        org_with_colliding_slug = self.create_organization(
-            slug=desired_slug, name="collision-imminent"
-        )
-
-        with assume_test_silo_mode(SiloMode.CONTROL):
-            colliding_org_mapping = OrganizationMapping.objects.get(
-                organization_id=org_with_colliding_slug.id
-            )
-        assert colliding_org_mapping.slug == desired_slug
-
-        # Queue a slug update but don't drain the shard yet to ensure a temporary collision happens
-        org_with_colliding_slug.slug = "unique-slug"
-        with outbox_context(flush=False):
-            org_with_colliding_slug.save()
-
-        self.get_success_response(self.organization.slug, slug=desired_slug)
-        self.organization.refresh_from_db()
-        assert self.organization.slug == desired_slug
-
-        # Ensure that the organization update has been flushed, but it collides when attempting an upsert
-        with pytest.raises(OutboxFlushError):
-            self.organization.outbox_for_update().drain_shard()
-
-        with assume_test_silo_mode(SiloMode.CONTROL):
-            organization_mapping = OrganizationMapping.objects.get(
-                organization_id=self.organization.id
-            )
-        assert organization_mapping.slug == previous_slug
-
-        # Flush the colliding org slug change
-        org_with_colliding_slug.outbox_for_update().drain_shard()
-        colliding_org_mapping.refresh_from_db()
-        assert colliding_org_mapping.slug == "unique-slug"
-
-        # Flush the desired slug change and assert the correct slug was resolved
-        self.organization.outbox_for_update().drain_shard()
-        organization_mapping.refresh_from_db()
-        assert organization_mapping.slug == desired_slug
-
     def test_org_mapping_already_taken(self):
         self.create_organization(slug="taken")
         self.get_error_response(self.organization.slug, slug="taken", status_code=400)

+ 2 - 2
tests/sentry/hybrid_cloud/test_organization_provisioning.py

@@ -11,11 +11,11 @@ from sentry.models import (
     outbox_context,
 )
 from sentry.services.hybrid_cloud.organization import RpcOrganization
-from sentry.services.hybrid_cloud.organization_provisioning import (
+from sentry.services.hybrid_cloud.organization_provisioning import organization_provisioning_service
+from sentry.services.organization import (
     OrganizationOptions,
     OrganizationProvisioningOptions,
     PostProvisionOptions,
-    organization_provisioning_service,
 )
 from sentry.silo import SiloMode
 from sentry.testutils.cases import TestCase