Browse Source

ref(hc): Split HC services' init modules (#48671)

This is a code reorganization aimed primarily at reducing the risk of
circular imports.

From each service package's `__init__` module, move the definitions of
the serializable RPC model classes to a `model` module, and move the
definitions of the RPC service interface to a `service` module. This has
the benefit of reducing the total number of import statements in each.
Both namespaces are still exported from `__init__`, but importing the
individual submodules could further reduce exposure to circular imports.

For all service class methods that serialize ORM objects into RPC
models, move them to a new `serial` module as stand-alone functions.
This has the further benefit of removing the imports of the ORM classes,
which are particularly common causes of circular imports. (The `serial`
modules mostly need to be imported only into `impl`, where before ORM
classes were sometimes imported into `__init__`.)
Ryan Skonnord 1 year ago
parent
commit
207ca8dacc

+ 1 - 1
src/sentry/api/endpoints/integrations/sentry_apps/installation/external_issue/actions.py

@@ -7,7 +7,7 @@ from sentry.api.serializers import serialize
 from sentry.mediators.external_issues import IssueLinkCreator
 from sentry.models import Group, Project
 from sentry.models.user import User
-from sentry.services.hybrid_cloud.user.impl import serialize_rpc_user
+from sentry.services.hybrid_cloud.user.serial import serialize_rpc_user
 from sentry.utils.functional import extract_lazy_object
 
 

+ 3 - 7
src/sentry/auth/access.py

@@ -43,7 +43,7 @@ from sentry.services.hybrid_cloud.organization import (
     RpcUserOrganizationContext,
     organization_service,
 )
-from sentry.services.hybrid_cloud.organization.impl import DatabaseBackedOrganizationService
+from sentry.services.hybrid_cloud.organization.serial import summarize_member
 from sentry.services.hybrid_cloud.user import RpcUser, user_service
 from sentry.utils import metrics
 from sentry.utils.request_cache import request_cache
@@ -604,7 +604,7 @@ class OrganizationMemberAccess(DbAccess):
         auth_state = auth_service.get_user_auth_state(
             organization_id=member.organization_id,
             is_superuser=False,
-            org_member=DatabaseBackedOrganizationService.summarize_member(member),
+            org_member=summarize_member(member),
             user_id=member.user_id,
         )
         sso_state = auth_state.sso_state
@@ -997,11 +997,7 @@ def from_request(
             user_id=request.user.id,
             organization_id=organization.id,
             is_superuser=is_superuser,
-            org_member=(
-                DatabaseBackedOrganizationService.summarize_member(member)
-                if member is not None
-                else None
-            ),
+            org_member=(summarize_member(member) if member is not None else None),
         ).sso_state
 
         return OrganizationGlobalAccess(

+ 2 - 2
src/sentry/auth/helper.py

@@ -41,7 +41,7 @@ from sentry.services.hybrid_cloud.organization import (
     RpcOrganizationMember,
     organization_service,
 )
-from sentry.services.hybrid_cloud.organization.impl import DatabaseBackedOrganizationService
+from sentry.services.hybrid_cloud.organization.serial import serialize_organization
 from sentry.signals import sso_enabled, user_signup
 from sentry.tasks.auth import email_missing_links
 from sentry.utils import auth, json, metrics
@@ -737,7 +737,7 @@ class AuthHelper(Pipeline):
     def auth_handler(self, identity: Mapping[str, Any]) -> AuthIdentityHandler:
         # This is a temporary step to keep test_helper integrated
         # TODO: Move this conversion further upstream
-        rpc_org = DatabaseBackedOrganizationService.serialize_organization(self.organization)
+        rpc_org = serialize_organization(self.organization)
 
         return AuthIdentityHandler(
             self.provider_model, self.provider, rpc_org, self.request, identity

+ 2 - 2
src/sentry/receivers/outbox/region.py

@@ -23,8 +23,8 @@ from sentry.receivers.outbox import maybe_process_tombstone
 from sentry.services.hybrid_cloud.identity import identity_service
 from sentry.services.hybrid_cloud.log import AuditLogEvent, UserIpEvent
 from sentry.services.hybrid_cloud.log.impl import DatabaseBackedLogService
-from sentry.services.hybrid_cloud.organization_mapping import (
-    organization_mapping_service,
+from sentry.services.hybrid_cloud.organization_mapping import organization_mapping_service
+from sentry.services.hybrid_cloud.organization_mapping.serial import (
     update_organization_mapping_from_instance,
 )
 from sentry.services.hybrid_cloud.organizationmember_mapping import (

+ 2 - 2
src/sentry/rules/actions/notify_event_service.py

@@ -19,7 +19,7 @@ from sentry.rules.actions.services import PluginService
 from sentry.rules.base import CallbackFuture
 from sentry.services.hybrid_cloud.app import RpcSentryAppService, app_service
 from sentry.services.hybrid_cloud.integration import integration_service
-from sentry.services.hybrid_cloud.organization.impl import DatabaseBackedOrganizationService
+from sentry.services.hybrid_cloud.organization.serial import serialize_organization
 from sentry.tasks.sentry_apps import notify_sentry_app
 from sentry.utils import metrics
 from sentry.utils.safe import safe_execute
@@ -63,7 +63,7 @@ def send_incident_alert_notification(
     fire. If not provided we'll attempt to calculate this ourselves.
     :return:
     """
-    organization = DatabaseBackedOrganizationService.serialize_organization(incident.organization)
+    organization = serialize_organization(incident.organization)
     incident_attachment = build_incident_attachment(incident, new_status, metric_value)
 
     integration_service.send_incident_alert_notification(

+ 2 - 4
src/sentry/sentry_apps/components.py

@@ -10,7 +10,7 @@ from django.utils.http import urlencode
 
 from sentry.mediators.external_requests import SelectRequester
 from sentry.models import SentryAppComponent, SentryAppInstallation
-from sentry.services.hybrid_cloud.app import app_service
+from sentry.services.hybrid_cloud.app.serial import serialize_sentry_app_installation
 from sentry.utils import json
 
 
@@ -101,9 +101,7 @@ class SentryAppComponentPreparer:
                 field.update(self._request(field["uri"], dependent_data=dependant_data))
 
     def _request(self, uri: str, dependent_data: str | None = None) -> Any:
-        install = app_service.serialize_sentry_app_installation(
-            self.install, self.install.sentry_app
-        )
+        install = serialize_sentry_app_installation(self.install, self.install.sentry_app)
         return SelectRequester.run(
             install=install,
             project_slug=self.project_slug,

+ 1 - 1
src/sentry/services/hybrid_cloud/actor.py

@@ -1,6 +1,6 @@
 # Please do not use
 #     from __future__ import annotations
-# in modules such as this one where hybrid cloud service classes and data models are
+# in modules such as this one where hybrid cloud data models or service classes are
 # defined, because we want to reflect on type annotations and avoid forward references.
 
 from enum import Enum

+ 2 - 289
src/sentry/services/hybrid_cloud/app/__init__.py

@@ -1,289 +1,2 @@
-# Please do not use
-#     from __future__ import annotations
-# in modules such as this one where hybrid cloud service classes and data models are
-# defined, because we want to reflect on type annotations and avoid forward references.
-
-import abc
-import datetime
-import hmac
-from dataclasses import dataclass
-from hashlib import sha256
-from typing import TYPE_CHECKING, Any, List, Mapping, Optional, Protocol, cast
-
-from pydantic.fields import Field
-from typing_extensions import TypedDict
-
-from sentry.constants import SentryAppInstallationStatus, SentryAppStatus
-from sentry.models import SentryApp, SentryAppInstallation
-from sentry.models.apiapplication import ApiApplication
-from sentry.services.hybrid_cloud import RpcModel
-from sentry.services.hybrid_cloud.filter_query import OpaqueSerializedResponse
-from sentry.services.hybrid_cloud.rpc import RpcService, rpc_method
-from sentry.services.hybrid_cloud.user import RpcUser
-from sentry.silo import SiloMode
-
-if TYPE_CHECKING:
-    from sentry.mediators.external_requests.alert_rule_action_requester import AlertRuleActionResult
-    from sentry.services.hybrid_cloud.auth import AuthenticationContext
-
-
-class RpcApiApplication(RpcModel):
-    id: int = -1
-    client_id: str = ""
-    client_secret: str = ""
-
-
-class RpcSentryAppService(RpcModel):
-    """
-    A `SentryAppService` (a notification service) wrapped up and serializable via the
-    rpc interface.
-    """
-
-    title: str = ""
-    slug: str = ""
-    service_type: str = "sentry_app"
-
-
-class RpcSentryApp(RpcModel):
-    id: int = -1
-    scope_list: List[str] = Field(default_factory=list)
-    application_id: int = -1
-    application: RpcApiApplication = Field(default_factory=RpcApiApplication)
-    proxy_user_id: Optional[int] = 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)
-    webhook_url: Optional[str] = None
-    is_published: bool = False
-    is_unpublished: bool = False
-    is_internal: bool = True
-    is_publish_request_inprogress: bool = False
-    status: str = ""
-
-    def show_auth_info(self, access: Any) -> bool:
-        encoded_scopes = set({"%s" % scope for scope in list(access.scopes)})
-        return set(self.scope_list).issubset(encoded_scopes)
-
-    def build_signature(self, body: str) -> str:
-        secret = self.application.client_secret
-        return hmac.new(
-            key=secret.encode("utf-8"), msg=body.encode("utf-8"), digestmod=sha256
-        ).hexdigest()
-
-    # Properties are copied from the sentry app ORM model.
-    @property
-    def slug_for_metrics(self) -> str:
-        if self.is_internal:
-            return "internal"
-        if self.is_unpublished:
-            return "unpublished"
-        return self.slug
-
-
-class RpcSentryAppInstallation(RpcModel):
-    id: int = -1
-    organization_id: int = -1
-    status: int = SentryAppInstallationStatus.PENDING
-    sentry_app: RpcSentryApp = Field(default_factory=lambda: RpcSentryApp())
-    date_deleted: Optional[datetime.datetime] = None
-    uuid: str = ""
-
-
-class RpcSentryAppComponent(RpcModel):
-    uuid: str = ""
-    sentry_app_id: int = -1
-    type: str = ""
-    app_schema: Mapping[str, Any] = Field(default_factory=dict)
-
-
-class SentryAppEventDataInterface(Protocol):
-    """
-    Protocol making RpcSentryAppEvents capable of consuming from various sources, keeping only
-    the minimum required properties.
-    """
-
-    id: str
-    label: str
-
-    @property
-    def actionType(self) -> str:
-        pass
-
-    def is_enabled(self) -> bool:
-        pass
-
-
-@dataclass  # TODO: Make compatible with RpcModel
-class RpcSentryAppEventData(SentryAppEventDataInterface):
-    id: str = ""
-    label: str = ""
-    action_type: str = ""
-    enabled: bool = True
-
-    @property
-    def actionType(self) -> str:
-        return self.action_type
-
-    def is_enabled(self) -> bool:
-        return self.enabled
-
-    @classmethod
-    def from_event(cls, data_interface: SentryAppEventDataInterface) -> "RpcSentryAppEventData":
-        return RpcSentryAppEventData(
-            id=data_interface.id,
-            label=data_interface.label,
-            action_type=data_interface.actionType,
-            enabled=data_interface.is_enabled(),
-        )
-
-
-class SentryAppInstallationFilterArgs(TypedDict, total=False):
-    installation_ids: List[int]
-    app_ids: List[int]
-    organization_id: int
-    uuids: List[str]
-
-
-class AppService(RpcService):
-    key = "app"
-    local_mode = SiloMode.CONTROL
-
-    @classmethod
-    def get_local_implementation(cls) -> RpcService:
-        from sentry.services.hybrid_cloud.app.impl import DatabaseBackedAppService
-
-        return DatabaseBackedAppService()
-
-    @rpc_method
-    @abc.abstractmethod
-    def serialize_many(
-        self,
-        *,
-        filter: SentryAppInstallationFilterArgs,
-        as_user: Optional[RpcUser] = None,
-        auth_context: Optional["AuthenticationContext"] = None,
-    ) -> List[OpaqueSerializedResponse]:
-        pass
-
-    @rpc_method
-    @abc.abstractmethod
-    def get_many(
-        self, *, filter: SentryAppInstallationFilterArgs
-    ) -> List[RpcSentryAppInstallation]:
-        pass
-
-    @rpc_method
-    @abc.abstractmethod
-    def find_installation_by_proxy_user(
-        self, *, proxy_user_id: int, organization_id: int
-    ) -> Optional[RpcSentryAppInstallation]:
-        pass
-
-    @rpc_method
-    @abc.abstractmethod
-    def get_installed_for_organization(
-        self,
-        *,
-        organization_id: int,
-    ) -> List[RpcSentryAppInstallation]:
-        pass
-
-    @rpc_method
-    @abc.abstractmethod
-    def get_sentry_app_by_slug(self, *, slug: str) -> Optional[RpcSentryApp]:
-        pass
-
-    @rpc_method
-    @abc.abstractmethod
-    def find_alertable_services(self, *, organization_id: int) -> List[RpcSentryAppService]:
-        pass
-
-    @classmethod
-    def serialize_sentry_app(cls, app: SentryApp) -> RpcSentryApp:
-        return RpcSentryApp(
-            id=app.id,
-            scope_list=app.scope_list,
-            application_id=app.application_id,
-            application=cls.serialize_api_application(app.application),
-            proxy_user_id=app.proxy_user_id,
-            owner_id=app.owner_id,
-            name=app.name,
-            slug=app.slug,
-            uuid=app.uuid,
-            events=app.events,
-            webhook_url=app.webhook_url,
-            is_published=app.status == SentryAppStatus.PUBLISHED,
-            is_unpublished=app.status == SentryAppStatus.UNPUBLISHED,
-            is_internal=app.status == SentryAppStatus.INTERNAL,
-            is_publish_request_inprogress=app.status == SentryAppStatus.PUBLISH_REQUEST_INPROGRESS,
-            status=app.status,
-        )
-
-    @classmethod
-    def serialize_api_application(self, api_app: ApiApplication) -> RpcApiApplication:
-        return RpcApiApplication(
-            id=api_app.id,
-            client_id=api_app.client_id,
-            client_secret=api_app.client_secret,
-        )
-
-    @rpc_method
-    @abc.abstractmethod
-    def find_service_hook_sentry_app(self, *, api_application_id: int) -> Optional[RpcSentryApp]:
-        pass
-
-    @rpc_method
-    @abc.abstractmethod
-    def get_custom_alert_rule_actions(
-        self,
-        *,
-        event_data: RpcSentryAppEventData,
-        organization_id: int,
-        project_slug: Optional[str],
-    ) -> List[Mapping[str, Any]]:
-        pass
-
-    @rpc_method
-    @abc.abstractmethod
-    def find_app_components(self, *, app_id: int) -> List[RpcSentryAppComponent]:
-        pass
-
-    @rpc_method
-    @abc.abstractmethod
-    def get_related_sentry_app_components(
-        self,
-        *,
-        organization_ids: List[int],
-        sentry_app_ids: List[int],
-        type: str,
-        group_by: str = "sentry_app_id",
-    ) -> Mapping[str, Any]:
-        pass
-
-    @classmethod
-    def serialize_sentry_app_installation(
-        cls, installation: SentryAppInstallation, app: Optional[SentryApp] = None
-    ) -> RpcSentryAppInstallation:
-        if app is None:
-            app = installation.sentry_app
-
-        return RpcSentryAppInstallation(
-            id=installation.id,
-            organization_id=installation.organization_id,
-            status=installation.status,
-            sentry_app=cls.serialize_sentry_app(app),
-            date_deleted=installation.date_deleted,
-            uuid=installation.uuid,
-        )
-
-    @rpc_method
-    @abc.abstractmethod
-    def trigger_sentry_app_action_creators(
-        self, *, fields: List[Mapping[str, Any]], install_uuid: Optional[str]
-    ) -> "AlertRuleActionResult":
-        pass
-
-
-app_service = cast(AppService, AppService.create_delegation())
+from .model import *  # noqa
+from .service import *  # noqa

+ 8 - 6
src/sentry/services/hybrid_cloud/app/impl.py

@@ -19,6 +19,10 @@ from sentry.services.hybrid_cloud.app import (
     RpcSentryAppService,
     SentryAppInstallationFilterArgs,
 )
+from sentry.services.hybrid_cloud.app.serial import (
+    serialize_sentry_app,
+    serialize_sentry_app_installation,
+)
 from sentry.services.hybrid_cloud.auth import AuthenticationContext
 from sentry.services.hybrid_cloud.filter_query import (
     FilterQueryDatabaseImpl,
@@ -56,7 +60,7 @@ class DatabaseBackedAppService(AppService):
     def get_sentry_app_by_slug(self, *, slug: str) -> Optional[RpcSentryApp]:
         try:
             sentry_app = SentryApp.objects.get(slug=slug)
-            return self.serialize_sentry_app(sentry_app)
+            return serialize_sentry_app(sentry_app)
         except SentryApp.DoesNotExist:
             return None
 
@@ -156,7 +160,7 @@ class DatabaseBackedAppService(AppService):
             return query
 
         def serialize_rpc(self, object: SentryAppInstallation) -> RpcSentryAppInstallation:
-            return AppService.serialize_sentry_app_installation(object)
+            return serialize_sentry_app_installation(object)
 
     _FQ = _AppServiceFilterQuery()
 
@@ -175,7 +179,7 @@ class DatabaseBackedAppService(AppService):
         except SentryAppInstallation.DoesNotExist:
             return None
 
-        return self.serialize_sentry_app_installation(installation, sentry_app)
+        return serialize_sentry_app_installation(installation, sentry_app)
 
     def trigger_sentry_app_action_creators(
         self, *, fields: List[Mapping[str, Any]], install_uuid: str | None
@@ -192,9 +196,7 @@ class DatabaseBackedAppService(AppService):
 
     def find_service_hook_sentry_app(self, *, api_application_id: int) -> Optional[RpcSentryApp]:
         try:
-            return self.serialize_sentry_app(
-                SentryApp.objects.get(application_id=api_application_id)
-            )
+            return serialize_sentry_app(SentryApp.objects.get(application_id=api_application_id))
         except SentryApp.DoesNotExist:
             return None
 

+ 135 - 0
src/sentry/services/hybrid_cloud/app/model.py

@@ -0,0 +1,135 @@
+# Please do not use
+#     from __future__ import annotations
+# in modules such as this one where hybrid cloud data models or service classes are
+# defined, because we want to reflect on type annotations and avoid forward references.
+
+import datetime
+import hmac
+from dataclasses import dataclass
+from hashlib import sha256
+from typing import Any, List, Mapping, Optional, Protocol
+
+from pydantic.fields import Field
+from typing_extensions import TypedDict
+
+from sentry.constants import SentryAppInstallationStatus
+from sentry.services.hybrid_cloud import RpcModel
+
+
+class RpcApiApplication(RpcModel):
+    id: int = -1
+    client_id: str = ""
+    client_secret: str = ""
+
+
+class RpcSentryAppService(RpcModel):
+    """
+    A `SentryAppService` (a notification service) wrapped up and serializable via the
+    rpc interface.
+    """
+
+    title: str = ""
+    slug: str = ""
+    service_type: str = "sentry_app"
+
+
+class RpcSentryApp(RpcModel):
+    id: int = -1
+    scope_list: List[str] = Field(default_factory=list)
+    application_id: int = -1
+    application: RpcApiApplication = Field(default_factory=RpcApiApplication)
+    proxy_user_id: Optional[int] = 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)
+    webhook_url: Optional[str] = None
+    is_published: bool = False
+    is_unpublished: bool = False
+    is_internal: bool = True
+    is_publish_request_inprogress: bool = False
+    status: str = ""
+
+    def show_auth_info(self, access: Any) -> bool:
+        encoded_scopes = set({"%s" % scope for scope in list(access.scopes)})
+        return set(self.scope_list).issubset(encoded_scopes)
+
+    def build_signature(self, body: str) -> str:
+        secret = self.application.client_secret
+        return hmac.new(
+            key=secret.encode("utf-8"), msg=body.encode("utf-8"), digestmod=sha256
+        ).hexdigest()
+
+    # Properties are copied from the sentry app ORM model.
+    @property
+    def slug_for_metrics(self) -> str:
+        if self.is_internal:
+            return "internal"
+        if self.is_unpublished:
+            return "unpublished"
+        return self.slug
+
+
+class RpcSentryAppInstallation(RpcModel):
+    id: int = -1
+    organization_id: int = -1
+    status: int = SentryAppInstallationStatus.PENDING
+    sentry_app: RpcSentryApp = Field(default_factory=lambda: RpcSentryApp())
+    date_deleted: Optional[datetime.datetime] = None
+    uuid: str = ""
+
+
+class RpcSentryAppComponent(RpcModel):
+    uuid: str = ""
+    sentry_app_id: int = -1
+    type: str = ""
+    app_schema: Mapping[str, Any] = Field(default_factory=dict)
+
+
+class SentryAppEventDataInterface(Protocol):
+    """
+    Protocol making RpcSentryAppEvents capable of consuming from various sources, keeping only
+    the minimum required properties.
+    """
+
+    id: str
+    label: str
+
+    @property
+    def actionType(self) -> str:
+        pass
+
+    def is_enabled(self) -> bool:
+        pass
+
+
+@dataclass  # TODO: Make compatible with RpcModel
+class RpcSentryAppEventData(SentryAppEventDataInterface):
+    id: str = ""
+    label: str = ""
+    action_type: str = ""
+    enabled: bool = True
+
+    @property
+    def actionType(self) -> str:
+        return self.action_type
+
+    def is_enabled(self) -> bool:
+        return self.enabled
+
+    @classmethod
+    def from_event(cls, data_interface: SentryAppEventDataInterface) -> "RpcSentryAppEventData":
+        return RpcSentryAppEventData(
+            id=data_interface.id,
+            label=data_interface.label,
+            action_type=data_interface.actionType,
+            enabled=data_interface.is_enabled(),
+        )
+
+
+class SentryAppInstallationFilterArgs(TypedDict, total=False):
+    installation_ids: List[int]
+    app_ids: List[int]
+    organization_id: int
+    uuids: List[str]

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