Skip to content

Commit

Permalink
ref(mediators): Convert SentryAppInstallation mediators to dataclasses (
Browse files Browse the repository at this point in the history
  • Loading branch information
Christinarlong authored Oct 9, 2024
1 parent abfe52e commit 70062c3
Show file tree
Hide file tree
Showing 14 changed files with 167 additions and 134 deletions.
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,6 @@ module = [
"sentry.issues.update_inbox",
"sentry.lang.java.processing",
"sentry.llm.*",
"sentry.mediators.sentry_app_installations.installation_notifier",
"sentry.migrations.*",
"sentry.models.event",
"sentry.models.eventattachment",
Expand Down
1 change: 0 additions & 1 deletion src/sentry/mediators/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from .mediator import Mediator # NOQA
from .param import Param # NOQA
from .sentry_app_installations import * # NOQA
from .token_exchange.grant_exchanger import GrantExchanger # noqa: F401
from .token_exchange.refresher import Refresher # noqa: F401
from .token_exchange.util import AUTHORIZATION, REFRESH, GrantTypes # noqa: F401
2 changes: 0 additions & 2 deletions src/sentry/mediators/sentry_app_installations/__init__.py

This file was deleted.

This file was deleted.

32 changes: 0 additions & 32 deletions src/sentry/mediators/sentry_app_installations/updater.py

This file was deleted.

35 changes: 22 additions & 13 deletions src/sentry/sentry_apps/api/endpoints/installation_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import control_silo_endpoint
from sentry.api.serializers import serialize
from sentry.mediators.sentry_app_installations.installation_notifier import InstallationNotifier
from sentry.mediators.sentry_app_installations.updater import Updater
from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint
from sentry.sentry_apps.api.parsers.sentry_app_installation import SentryAppInstallationParser
from sentry.sentry_apps.api.serializers.sentry_app_installation import (
SentryAppInstallationSerializer,
)
from sentry.sentry_apps.installations import (
SentryAppInstallationNotifier,
SentryAppInstallationUpdater,
)
from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
from sentry.utils.audit import create_audit_entry

Expand All @@ -39,26 +41,33 @@ def get(self, request: Request, installation) -> Response:
)

def delete(self, request: Request, installation) -> Response:
installation = SentryAppInstallation.objects.get(id=installation.id)
sentry_app_installation = SentryAppInstallation.objects.get(id=installation.id)
with transaction.atomic(using=router.db_for_write(SentryAppInstallation)):
try:
InstallationNotifier.run(install=installation, user=request.user, action="deleted")
assert (
request.user.is_authenticated
), "User must be authenticated to delete installation"
SentryAppInstallationNotifier(
sentry_app_installation=sentry_app_installation,
user=request.user,
action="deleted",
).run()
# if the error is from a request exception, log the error and continue
except RequestException as exc:
sentry_sdk.capture_exception(exc)
deletions.exec_sync(installation)
deletions.exec_sync(sentry_app_installation)
create_audit_entry(
request=request,
organization_id=installation.organization_id,
target_object=installation.organization_id,
organization_id=sentry_app_installation.organization_id,
target_object=sentry_app_installation.organization_id,
event=audit_log.get_event_id("SENTRY_APP_UNINSTALL"),
data={"sentry_app": installation.sentry_app.name},
data={"sentry_app": sentry_app_installation.sentry_app.name},
)
analytics.record(
"sentry_app.uninstalled",
user_id=request.user.id,
organization_id=installation.organization_id,
sentry_app=installation.sentry_app.slug,
organization_id=sentry_app_installation.organization_id,
sentry_app=sentry_app_installation.sentry_app.slug,
)
return Response(status=204)

Expand All @@ -68,9 +77,9 @@ def put(self, request: Request, installation) -> Response:
if serializer.is_valid():
result = serializer.validated_data

Updater.run(
user=request.user, sentry_app_installation=installation, status=result.get("status")
)
SentryAppInstallationUpdater(
sentry_app_installation=installation, status=result.get("status")
).run()

return Response(
serialize(
Expand Down
11 changes: 7 additions & 4 deletions src/sentry/sentry_apps/api/endpoints/sentry_app_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from sentry.api.serializers import serialize
from sentry.auth.staff import is_active_staff
from sentry.constants import SentryAppStatus
from sentry.mediators.sentry_app_installations.installation_notifier import InstallationNotifier
from sentry.organizations.services.organization import organization_service
from sentry.sentry_apps.api.bases.sentryapps import (
SentryAppAndStaffPermission,
Expand All @@ -25,6 +24,7 @@
from sentry.sentry_apps.api.serializers.sentry_app import (
SentryAppSerializer as ResponseSentryAppSerializer,
)
from sentry.sentry_apps.installations import SentryAppInstallationNotifier
from sentry.sentry_apps.logic import SentryAppUpdater
from sentry.sentry_apps.models.sentry_app import SentryApp
from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
Expand Down Expand Up @@ -162,9 +162,12 @@ def delete(self, request: Request, sentry_app) -> Response:
for install in sentry_app.installations.all():
try:
with transaction.atomic(using=router.db_for_write(SentryAppInstallation)):
InstallationNotifier.run(
install=install, user=request.user, action="deleted"
)
assert (
request.user.is_authenticated
), "User must be authenticated to delete installation"
SentryAppInstallationNotifier(
sentry_app_installation=install, user=request.user, action="deleted"
).run()
deletions.exec_sync(install)
except RequestException as exc:
sentry_sdk.capture_exception(exc)
Expand Down
75 changes: 75 additions & 0 deletions src/sentry/sentry_apps/installations.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,17 @@
from django.http.request import HttpRequest

from sentry import analytics, audit_log
from sentry.api.serializers import serialize
from sentry.constants import INTERNAL_INTEGRATION_TOKEN_COUNT_MAX, SentryAppInstallationStatus
from sentry.coreapi import APIUnauthorized
from sentry.exceptions import ApiTokenLimitError
from sentry.models.apiapplication import ApiApplication
from sentry.models.apigrant import ApiGrant
from sentry.models.apitoken import ApiToken
from sentry.sentry_apps.api.serializers.app_platform_event import AppPlatformEvent
from sentry.sentry_apps.api.serializers.sentry_app_installation import (
SentryAppInstallationSerializer,
)
from sentry.sentry_apps.models.sentry_app import SentryApp
from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
from sentry.sentry_apps.models.sentry_app_installation_token import SentryAppInstallationToken
Expand All @@ -21,6 +27,9 @@
from sentry.users.models.user import User
from sentry.users.services.user.model import RpcUser
from sentry.utils import metrics
from sentry.utils.sentry_apps import send_and_save_webhook_request

VALID_ACTIONS = ["created", "deleted"]


@dataclasses.dataclass
Expand Down Expand Up @@ -174,3 +183,69 @@ def api_application(self) -> ApiApplication:
@cached_property
def sentry_app(self) -> SentryApp:
return SentryApp.objects.get(slug=self.slug)


@dataclasses.dataclass
class SentryAppInstallationNotifier:
sentry_app_installation: SentryAppInstallation
user: User | RpcUser
action: str

def run(self) -> None:
if self.action not in VALID_ACTIONS:
raise APIUnauthorized(
f"Invalid action '{self.action} for installation notifier for {self.sentry_app}"
)

send_and_save_webhook_request(self.sentry_app, self.request)

@property
def request(self) -> AppPlatformEvent:
data = serialize(
self.sentry_app_installation,
user=self.user,
serializer=SentryAppInstallationSerializer(),
is_webhook=True,
)

return AppPlatformEvent(
resource="installation",
action=self.action,
install=self.sentry_app_installation,
data={"installation": data},
actor=self.user,
)

@cached_property
def sentry_app(self) -> SentryApp:
return self.sentry_app_installation.sentry_app

@cached_property
def api_grant(self) -> ApiGrant | None:
return self.sentry_app_installation.api_grant_id and self.sentry_app_installation.api_grant


@dataclasses.dataclass
class SentryAppInstallationUpdater:
sentry_app_installation: SentryAppInstallation
status: str | None = None

def run(self) -> SentryAppInstallation:
with transaction.atomic(router.db_for_write(SentryAppInstallation)):
self._update_status()
self.record_analytics()
return self.sentry_app_installation

def _update_status(self):
# convert from string to integer
if self.status == SentryAppInstallationStatus.INSTALLED_STR:
for install in SentryAppInstallation.objects.filter(id=self.sentry_app_installation.id):
install.update(status=SentryAppInstallationStatus.INSTALLED)

def record_analytics(self):
analytics.record(
"sentry_app_installation.updated",
sentry_app_installation_id=self.sentry_app_installation.id,
sentry_app_id=self.sentry_app_installation.sentry_app.id,
organization_id=self.sentry_app_installation.organization_id,
)
6 changes: 4 additions & 2 deletions src/sentry/sentry_apps/tasks/sentry_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def process_resource_change_bound(
)
@retry_decorator
def installation_webhook(installation_id: int, user_id: int, *args: Any, **kwargs: Any) -> None:
from sentry.mediators.sentry_app_installations.installation_notifier import InstallationNotifier
from sentry.sentry_apps.installations import SentryAppInstallationNotifier

extra = {"installation_id": installation_id, "user_id": user_id}
try:
Expand All @@ -278,7 +278,9 @@ def installation_webhook(installation_id: int, user_id: int, *args: Any, **kwarg
logger.info("installation_webhook.missing_user", extra=extra)
return

InstallationNotifier.run(install=install, user=user, action="created")
SentryAppInstallationNotifier(
sentry_app_installation=install, user=user, action="created"
).run()


@instrumented_task(
Expand Down
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from sentry.testutils.cases import APITestCase
from sentry.testutils.silo import control_silo_test
from sentry.users.services.user.service import user_service
from sentry.utils import json


class SentryAppInstallationDetailsTest(APITestCase):
Expand Down Expand Up @@ -101,24 +102,30 @@ def test_no_access_outside_install_organization(self):
@control_silo_test
class DeleteSentryAppInstallationDetailsTest(SentryAppInstallationDetailsTest):
@responses.activate
@patch("sentry.mediators.sentry_app_installations.InstallationNotifier.run")
@patch("sentry.analytics.record")
def test_delete_install(self, record, run):
def test_delete_install(self, record):
responses.add(url="https://example.com/webhook", method=responses.POST, body=b"")
self.login_as(user=self.user)
rpc_user = user_service.get_user(user_id=self.user.id)
assert rpc_user, "User should exist in test to delete sentry app installation unless noted"

response = self.client.delete(self.url, format="json")
assert AuditLogEntry.objects.filter(
event=audit_log.get_event_id("SENTRY_APP_UNINSTALL")
).exists()
run.assert_called_once_with(install=self.orm_installation2, user=rpc_user, action="deleted")
record.assert_called_with(
"sentry_app.uninstalled",
user_id=self.user.id,
organization_id=self.org.id,
sentry_app=self.orm_installation2.sentry_app.slug,
)

response_body = json.loads(responses.calls[0].request.body)

assert response_body.get("installation").get("uuid") == self.orm_installation2.uuid
assert response_body.get("action") == "deleted"
assert response_body.get("actor")["id"] == rpc_user.id

assert response.status_code == 204

def test_member_cannot_delete_install(self):
Expand Down
Loading

0 comments on commit 70062c3

Please sign in to comment.