chore(hybrid-cloud): reorganize services (#41557)

To reduce import cycles, split the service definitions from the
implementations.
Adds test utility that counts and caps service calls.
Also forces stubs to declare their target silo.

src/sentry/models/actor.py

@@ -6,10 +6,10 @@ from django.db.models.signals import pre_save
 from rest_framework import serializers
 
 from sentry.db.models import Model, region_silo_only_model
+from sentry.services.hybrid_cloud.user import APIUser, user_service
 
 if TYPE_CHECKING:
     from sentry.models import Team
-    from sentry.services.hybrid_cloud.user import APIUser
 
 ACTOR_TYPES = {"team": 0, "user": 1}
 
@@ -25,7 +25,6 @@ def actor_type_to_class(type: int) -> Type[Union["Team", "APIUser"]]:
 
 def fetch_actor_by_actor_id(cls, actor_id: int) -> Union["Team", "APIUser"]:
     from sentry.models import Team, User
-    from sentry.services.hybrid_cloud.user import user_service
 
     if cls is User:
         user = user_service.get_by_actor_id(actor_id)
@@ -45,7 +44,6 @@ def fetch_actor_by_id(cls, id: int) -> Union["Team", "APIUser"]:
         return Team.objects.get(id=id)
 
     if cls is User:
-        from sentry.services.hybrid_cloud.user import user_service
 
         user = user_service.get_user(id)
         if user is None:
@@ -153,7 +151,6 @@ class ActorTuple(namedtuple("Actor", "id type")):
         :return:
         """
         from sentry.models import User
-        from sentry.services.hybrid_cloud.user import user_service
 
         if not actors:
             return []

src/sentry/models/commitauthor.py

@@ -1,12 +1,10 @@
-from typing import TYPE_CHECKING, List
+from typing import List
 
 from django.db import models
 
 from sentry.db.models import BoundedBigIntegerField, Model, region_silo_only_model, sane_repr
 from sentry.db.models.manager import BaseManager
-
-if TYPE_CHECKING:
-    from sentry.services.hybrid_cloud.user import APIUser
+from sentry.services.hybrid_cloud.user import APIUser, user_service
 
 
 class CommitAuthorManager(BaseManager):
@@ -39,7 +37,6 @@ class CommitAuthor(Model):
 
     def find_users(self) -> List["APIUser"]:
         from sentry.models import OrganizationMember
-        from sentry.services.hybrid_cloud.user import user_service
 
         users = user_service.get_many_by_email(self.email)
         org_member_user_ids = set(

src/sentry/models/groupsubscription.py

@@ -18,11 +18,11 @@ from sentry.notifications.helpers import (
     where_should_be_participating,
 )
 from sentry.notifications.types import GroupSubscriptionReason, NotificationSettingTypes
+from sentry.services.hybrid_cloud.user import APIUser, user_service
 from sentry.types.integrations import ExternalProviders
 
 if TYPE_CHECKING:
     from sentry.models import Group, Team, User
-    from sentry.services.hybrid_cloud.user import APIUser
 
 
 class GroupSubscriptionManager(BaseManager):  # type: ignore
@@ -56,9 +56,8 @@ class GroupSubscriptionManager(BaseManager):  # type: ignore
         reason: int = GroupSubscriptionReason.unknown,
     ) -> Optional[bool]:
         from sentry.models import Team, User
-        from sentry.services.hybrid_cloud.user import APIUser as APIUserClass
 
-        if isinstance(actor, APIUserClass) or isinstance(actor, User):
+        if isinstance(actor, APIUser) or isinstance(actor, User):
             return self.subscribe(group, actor, reason)
         if isinstance(actor, Team):
             # subscribe the members of the team
@@ -119,7 +118,6 @@ class GroupSubscriptionManager(BaseManager):  # type: ignore
         :param group: Group object
         """
         from sentry.models import NotificationSetting
-        from sentry.services.hybrid_cloud.user import user_service
 
         all_possible_users = user_service.get_from_group(group)
         active_and_disabled_subscriptions = self.filter(

src/sentry/models/lostpasswordhash.py

@@ -11,7 +11,7 @@ from sentry.utils.http import absolute_uri
 from sentry.utils.security import get_secure_token
 
 if TYPE_CHECKING:
-    from sentry.services.hybrid_cloud.lostpasswordhash import APILostPasswordHash
+    from sentry.services.hybrid_cloud.lost_password_hash import APILostPasswordHash
 
 
 @control_silo_only_model
@@ -78,7 +78,7 @@ class LostPasswordHash(Model):
 
     @classmethod
     def for_user(cls, user) -> "APILostPasswordHash":
-        from sentry.services.hybrid_cloud.lostpasswordhash import lost_password_hash_service
+        from sentry.services.hybrid_cloud.lost_password_hash import lost_password_hash_service
 
         password_hash = lost_password_hash_service.get_or_create(user.id)
         return password_hash

src/sentry/models/options/user_option.py

@@ -13,7 +13,6 @@ if TYPE_CHECKING:
     from sentry.models import Organization, Project, User
     from sentry.services.hybrid_cloud.user import APIUser
 
-
 option_scope_error = "this is not a supported use case, scope to project OR organization"
 
 

src/sentry/models/organization.py

@@ -3,7 +3,7 @@ from __future__ import annotations
 import logging
 from datetime import timedelta
 from enum import IntEnum
-from typing import TYPE_CHECKING, FrozenSet, Sequence
+from typing import FrozenSet, Sequence
 
 from django.conf import settings
 from django.db import IntegrityError, models, router, transaction
@@ -31,14 +31,11 @@ from sentry.db.models.utils import slugify_instance
 from sentry.locks import locks
 from sentry.models.organizationmember import OrganizationMember
 from sentry.roles.manager import Role
+from sentry.services.hybrid_cloud.user import APIUser, user_service
 from sentry.utils.http import absolute_uri
 from sentry.utils.retries import TimedRetryPolicy
 from sentry.utils.snowflake import SnowflakeIdMixin
 
-if TYPE_CHECKING:
-    from sentry.services.hybrid_cloud.user import APIUser
-
-
 SENTRY_USE_SNOWFLAKE = getattr(settings, "SENTRY_USE_SNOWFLAKE", False)
 
 
@@ -253,7 +250,6 @@ class Organization(Model, SnowflakeIdMixin):
         }
 
     def get_owners(self) -> Sequence[APIUser]:
-        from sentry.services.hybrid_cloud.user import user_service
 
         owner_memberships = OrganizationMember.objects.filter(
             role=roles.get_top_dog().id, organization=self

src/sentry/models/organizationmember.py

@@ -292,7 +292,7 @@ class OrganizationMember(Model):
         msg.send_async([self.get_email()])
 
     def send_sso_unlink_email(self, actor, provider):
-        from sentry.services.hybrid_cloud.lostpasswordhash import lost_password_hash_service
+        from sentry.services.hybrid_cloud.lost_password_hash import lost_password_hash_service
         from sentry.utils.email import MessageBuilder
 
         email = self.get_email()

src/sentry/services/hybrid_cloud/__init__.py

@@ -1,10 +1,9 @@
 from __future__ import annotations
 
 import contextlib
-import functools
 import logging
+import threading
 from abc import ABC, abstractmethod
-from types import TracebackType
 from typing import Any, Callable, Dict, Generator, Generic, Mapping, Optional, Type, TypeVar, cast
 
 logger = logging.getLogger(__name__)
@@ -70,29 +69,36 @@ class DelegatedBySiloMode(Generic[ServiceInterface]):
         self._singleton = {}
 
 
-def CreateStubFromBase(base: Type[ServiceInterface]) -> Type[ServiceInterface]:
+hc_test_stub: Any = threading.local()
+
+
+def CreateStubFromBase(
+    base: Type[ServiceInterface], target_mode: SiloMode
+) -> Type[ServiceInterface]:
     """
     Using a concrete implementation class of a service, creates a new concrete implementation class suitable for a test
     stub.  It retains parity with the given base by passing through all of its abstract method implementations to the
-    given base class, but wraps it with `exempt_from_silo_limits`, allowing tests written for monolith mode to largely
+    given base class, but wraps it to run in the target silo mode, allowing tests written for monolith mode to largely
     work symmetrically.  In the future, however, when monolith mode separate is deprecated, this logic should be
-    replaced by true mocking utilities.
+    replaced by true mocking utilities, for say, target RPC endpoints.
 
     This implementation will not work outside of test contexts.
     """
     Super = base.__bases__[0]
 
-    def __init__(self: Any, *args: Any, **kwds: Any) -> None:
-        self.backing_service = base(*args, **kwds)
+    def __init__(self: Any, backing_service: ServiceInterface) -> None:
+        self.backing_service = backing_service
 
     def close(self: Any) -> None:
         self.backing_service.close()
 
     def make_method(method_name: str) -> Any:
         def method(self: Any, *args: Any, **kwds: Any) -> Any:
-            from sentry.testutils.silo import exempt_from_silo_limits
+            from django.test import override_settings
 
-            with exempt_from_silo_limits():
+            with override_settings(SILO_MODE=target_mode):
+                if cb := getattr(hc_test_stub, "cb", None):
+                    cb(self.backing_service, method_name, *args, **kwds)
                 return getattr(self.backing_service, method_name)(*args, **kwds)
 
         return method
@@ -109,6 +115,14 @@ def CreateStubFromBase(base: Type[ServiceInterface]) -> Type[ServiceInterface]:
     return cast(Type[ServiceInterface], type(f"Stub{Super.__name__}", (Super,), methods))
 
 
+def stubbed(f: Callable[[], ServiceInterface], mode: SiloMode) -> Callable[[], ServiceInterface]:
+    def factory() -> ServiceInterface:
+        backing = f()
+        return cast(ServiceInterface, cast(Any, CreateStubFromBase(type(backing), mode))(backing))
+
+    return factory
+
+
 def silo_mode_delegation(
     mapping: Mapping[SiloMode, Callable[[], ServiceInterface]]
 ) -> ServiceInterface:
@@ -117,72 +131,3 @@ def silo_mode_delegation(
     the mapping values.
     """
     return cast(ServiceInterface, DelegatedBySiloMode(mapping))
-
-
-@contextlib.contextmanager
-def service_stubbed(
-    service: InterfaceWithLifecycle,
-    stub: Optional[InterfaceWithLifecycle],
-    silo_mode: Optional[SiloMode] = None,
-) -> Generator[None, None, None]:
-    """
-    Replaces a service created with silo_mode_delegation with a replacement implementation while inside of the scope,
-    closing the existing implementation on enter and closing the given implementation on exit.
-    """
-    if silo_mode is None:
-        silo_mode = SiloMode.get_current_mode()
-
-    if isinstance(service, DelegatedBySiloMode):
-        with service.with_replacement(stub, silo_mode):
-            yield
-    else:
-        raise ValueError("Service needs to be a DelegatedBySilMode object, but it was not!")
-
-
-class use_real_service:
-    service: InterfaceWithLifecycle
-    silo_mode: SiloMode | None
-    context: contextlib.ExitStack
-
-    def __init__(self, service: InterfaceWithLifecycle, silo_mode: SiloMode | None):
-        self.silo_mode = silo_mode
-        self.service = service
-        self.context = contextlib.ExitStack()
-
-    def __enter__(self) -> None:
-        from django.test import override_settings
-
-        if isinstance(self.service, DelegatedBySiloMode):
-            if self.silo_mode is not None:
-                self.context.enter_context(override_settings(SILO_MODE=self.silo_mode))
-                self.context.enter_context(
-                    cast(
-                        Any,
-                        self.service.with_replacement(None, self.silo_mode),
-                    )
-                )
-            else:
-                self.context.enter_context(
-                    cast(
-                        Any,
-                        self.service.with_replacement(None, SiloMode.get_current_mode()),
-                    )
-                )
-        else:
-            raise ValueError("Service needs to be a DelegatedBySiloMode object, but it was not!")
-
-    def __call__(self, f: Callable[..., Any]) -> Callable[..., Any]:
-        @functools.wraps(f)
-        def wrapped(*args: Any, **kwds: Any) -> Any:
-            with use_real_service(self.service, self.silo_mode):
-                return f(*args, **kwds)
-
-        return wrapped
-
-    def __exit__(
-        self,
-        __exc_type: Type[BaseException] | None,
-        __exc_value: BaseException | None,
-        __traceback: TracebackType | None,
-    ) -> bool | None:
-        return self.context.__exit__(__exc_type, __exc_value, __traceback)

src/sentry/services/hybrid_cloud/app.py → src/sentry/services/hybrid_cloud/app/__init__.py

@@ -2,37 +2,14 @@ from __future__ import annotations
 
 import abc
 from dataclasses import dataclass, field
-from typing import List
+from typing import TYPE_CHECKING, List
 
 from sentry.constants import SentryAppInstallationStatus
-from sentry.models import SentryApp, SentryAppInstallation
-from sentry.services.hybrid_cloud import (
-    CreateStubFromBase,
-    InterfaceWithLifecycle,
-    silo_mode_delegation,
-)
+from sentry.services.hybrid_cloud import InterfaceWithLifecycle, silo_mode_delegation, stubbed
 from sentry.silo import SiloMode
 
-
-@dataclass
-class ApiSentryApp:
-    id: int = -1
-    scope_list: List[str] = field(default_factory=list)
-    application_id: int = -1
-    proxy_user_id: int | None = None  # can be null on deletion.
-    owner_id: int = -1  # relation to an organization
-    name: str = ""
-    slug: str = ""
-    uuid: str = ""
-    events: List[str] = field(default_factory=list)
-
-
-@dataclass
-class ApiSentryAppInstallation:
-    id: int = -1
-    organization_id: int = -1
-    status: int = SentryAppInstallationStatus.PENDING
-    sentry_app: ApiSentryApp = field(default_factory=lambda: ApiSentryApp())
+if TYPE_CHECKING:
+    from sentry.models import SentryApp, SentryAppInstallation
 
 
 class AppService(InterfaceWithLifecycle):
@@ -77,42 +54,37 @@ class AppService(InterfaceWithLifecycle):
         )
 
 
-class DatabaseBackedAppService(AppService):
-    def get_installed_for_organization(
-        self, *, organization_id: int
-    ) -> List[ApiSentryAppInstallation]:
-        installations = SentryAppInstallation.objects.get_installed_for_organization(
-            organization_id
-        ).select_related("sentry_app")
-        return [self.serialize_sentry_app_installation(i, i.sentry_app) for i in installations]
-
-    def find_installation_by_proxy_user(
-        self, *, proxy_user_id: int, organization_id: int
-    ) -> ApiSentryAppInstallation | None:
-        try:
-            sentry_app = SentryApp.objects.get(proxy_user_id=proxy_user_id)
-        except SentryApp.DoesNotExist:
-            return None
-
-        try:
-            installation = SentryAppInstallation.objects.get(
-                sentry_app_id=sentry_app.id, organization_id=organization_id
-            )
-        except SentryAppInstallation.DoesNotExist:
-            return None
-
-        return self.serialize_sentry_app_installation(installation, sentry_app)
+def impl_with_db() -> AppService:
+    from sentry.services.hybrid_cloud.app.impl import DatabaseBackedAppService
 
-    def close(self) -> None:
-        pass
+    return DatabaseBackedAppService()
 
 
-StubAppService = CreateStubFromBase(DatabaseBackedAppService)
-
 app_service: AppService = silo_mode_delegation(
     {
-        SiloMode.MONOLITH: lambda: DatabaseBackedAppService(),
-        SiloMode.CONTROL: lambda: DatabaseBackedAppService(),
-        SiloMode.REGION: lambda: StubAppService(),
+        SiloMode.MONOLITH: impl_with_db,
+        SiloMode.CONTROL: impl_with_db,
+        SiloMode.REGION: stubbed(impl_with_db, SiloMode.CONTROL),
     }
 )
+
+
+@dataclass
+class ApiSentryAppInstallation:
+    id: int = -1
+    organization_id: int = -1
+    status: int = SentryAppInstallationStatus.PENDING
+    sentry_app: ApiSentryApp = field(default_factory=lambda: ApiSentryApp())
+
+
+@dataclass
+class ApiSentryApp:
+    id: int = -1
+    scope_list: List[str] = field(default_factory=list)
+    application_id: int = -1
+    proxy_user_id: int | None = None  # can be null on deletion.
+    owner_id: int = -1  # relation to an organization
+    name: str = ""
+    slug: str = ""
+    uuid: str = ""
+    events: List[str] = field(default_factory=list)

src/sentry/services/hybrid_cloud/app/impl.py

@@ -0,0 +1,36 @@
+from __future__ import annotations
+
+from typing import List
+
+from sentry.models import SentryApp, SentryAppInstallation
+from sentry.services.hybrid_cloud.app import ApiSentryAppInstallation, AppService
+
+
+class DatabaseBackedAppService(AppService):
+    def get_installed_for_organization(
+        self, *, organization_id: int
+    ) -> List[ApiSentryAppInstallation]:
+        installations = SentryAppInstallation.objects.get_installed_for_organization(
+            organization_id
+        ).select_related("sentry_app")
+        return [self.serialize_sentry_app_installation(i, i.sentry_app) for i in installations]
+
+    def find_installation_by_proxy_user(
+        self, *, proxy_user_id: int, organization_id: int
+    ) -> ApiSentryAppInstallation | None:
+        try:
+            sentry_app = SentryApp.objects.get(proxy_user_id=proxy_user_id)
+        except SentryApp.DoesNotExist:
+            return None
+
+        try:
+            installation = SentryAppInstallation.objects.get(
+                sentry_app_id=sentry_app.id, organization_id=organization_id
+            )
+        except SentryAppInstallation.DoesNotExist:
+            return None
+
+        return self.serialize_sentry_app_installation(installation, sentry_app)
+
+    def close(self) -> None:
+        pass