diff --git a/src/sentry/hybridcloud/services/control_organization_provisioning/impl.py b/src/sentry/hybridcloud/services/control_organization_provisioning/impl.py index 0df1b35184b499..9253658e48cc0f 100644 --- a/src/sentry/hybridcloud/services/control_organization_provisioning/impl.py +++ b/src/sentry/hybridcloud/services/control_organization_provisioning/impl.py @@ -1,6 +1,6 @@ from copy import deepcopy -from django.db import router, transaction +from django.db import IntegrityError, router, transaction from django.utils import timezone as django_timezone from sentry import features, roles @@ -188,7 +188,7 @@ def update_organization_slug( organization_id: int, desired_slug: str, require_exact: bool = True, - ) -> RpcOrganizationSlugReservation: + ) -> RpcOrganizationSlugReservation | None: existing_slug_reservations = list( OrganizationSlugReservation.objects.filter(organization_id=organization_id) ) @@ -216,27 +216,34 @@ def update_organization_slug( if not require_exact: slug_base = self._generate_org_slug(region_name=region_name, slug=slug_base) - with outbox_context( - transaction.atomic(using=router.db_for_write(OrganizationSlugReservation)) - ): - OrganizationSlugReservation( - slug=slug_base, - organization_id=organization_id, - user_id=-1, - region_name=region_name, - reservation_type=OrganizationSlugReservationType.TEMPORARY_RENAME_ALIAS.value, - ).save(unsafe_write=True) - - org_mapping = OrganizationMapping.objects.filter( - organization_id=organization_id - ).first() - org = serialize_organization_mapping(org_mapping) if org_mapping is not None else None - if org and features.has("organizations:revoke-org-auth-on-slug-rename", org): - # Changing a slug invalidates all org tokens, so revoke them all. - auth_tokens = OrgAuthToken.objects.filter( - organization_id=organization_id, date_deactivated__isnull=True + try: + with outbox_context( + transaction.atomic(using=router.db_for_write(OrganizationSlugReservation)) + ): + OrganizationSlugReservation( + slug=slug_base, + organization_id=organization_id, + user_id=-1, + region_name=region_name, + reservation_type=OrganizationSlugReservationType.TEMPORARY_RENAME_ALIAS.value, + ).save(unsafe_write=True) + + org_mapping = OrganizationMapping.objects.filter( + organization_id=organization_id + ).first() + org = ( + serialize_organization_mapping(org_mapping) if org_mapping is not None else None ) - auth_tokens.update(date_deactivated=django_timezone.now()) + if org and features.has("organizations:revoke-org-auth-on-slug-rename", org): + # Changing a slug invalidates all org tokens, so revoke them all. + auth_tokens = OrgAuthToken.objects.filter( + organization_id=organization_id, date_deactivated__isnull=True + ) + auth_tokens.update(date_deactivated=django_timezone.now()) + except IntegrityError: + if require_exact: + return None + raise primary_slug = self._validate_primary_slug_updated( organization_id=organization_id, slug_base=slug_base diff --git a/src/sentry/hybridcloud/services/control_organization_provisioning/service.py b/src/sentry/hybridcloud/services/control_organization_provisioning/service.py index 3b33ea075700a7..7b1f4090636def 100644 --- a/src/sentry/hybridcloud/services/control_organization_provisioning/service.py +++ b/src/sentry/hybridcloud/services/control_organization_provisioning/service.py @@ -57,7 +57,7 @@ def update_organization_slug( organization_id: int, desired_slug: str, require_exact: bool = True, - ) -> RpcOrganizationSlugReservation: + ) -> RpcOrganizationSlugReservation | None: """ Updates an organization's slug via an outbox based confirmation flow to ensure that the control and region silos stay in sync. @@ -71,7 +71,7 @@ def update_organization_slug( :param desired_slug: The slug to update the organization with :param require_exact: Determines whether the slug can be modified with a unique suffix in the case of a slug collision. - :return: + :return: The updated slug reservation, or None if require_exact=True and the slug is already taken. """ @abstractmethod diff --git a/src/sentry/services/organization/provisioning.py b/src/sentry/services/organization/provisioning.py index ef6c4ab8c62bd2..4343c1894d2b34 100644 --- a/src/sentry/services/organization/provisioning.py +++ b/src/sentry/services/organization/provisioning.py @@ -114,6 +114,9 @@ def _control_based_slug_change( ) ) + if rpc_slug_reservation is None: + raise OrganizationSlugCollisionException + rpc_org = organization_service.get(id=rpc_slug_reservation.organization_id) if rpc_org is None: diff --git a/tests/sentry/hybridcloud/services/test_control_organization_provisioning.py b/tests/sentry/hybridcloud/services/test_control_organization_provisioning.py index d45f6786e920ef..46cc22059f4182 100644 --- a/tests/sentry/hybridcloud/services/test_control_organization_provisioning.py +++ b/tests/sentry/hybridcloud/services/test_control_organization_provisioning.py @@ -1,7 +1,7 @@ from __future__ import annotations import pytest -from django.db import IntegrityError, router, transaction +from django.db import router, transaction from django.db.models import QuerySet from sentry.hybridcloud.models.outbox import outbox_context @@ -251,22 +251,13 @@ def test_fails_to_update_exact_slug_with_collision(self) -> None: self.provisioning_args.provision_options.slug = conflicting_slug org_with_conflicting_slug = self.provision_organization() - if SiloMode.get_current_mode() == SiloMode.REGION: - with pytest.raises(RpcRemoteException): - control_organization_provisioning_rpc_service.update_organization_slug( - organization_id=test_org_slug_reservation.organization_id, - desired_slug=conflicting_slug, - require_exact=True, - region_name=self.region_name, - ) - else: - with pytest.raises(IntegrityError): - control_organization_provisioning_rpc_service.update_organization_slug( - organization_id=test_org_slug_reservation.organization_id, - desired_slug=conflicting_slug, - require_exact=True, - region_name=self.region_name, - ) + result = control_organization_provisioning_rpc_service.update_organization_slug( + organization_id=test_org_slug_reservation.organization_id, + desired_slug=conflicting_slug, + require_exact=True, + region_name=self.region_name, + ) + assert result is None with assume_test_silo_mode(SiloMode.CONTROL): org_slug_reservation = OrganizationSlugReservation.objects.get(