chore(actor) Remove AlertRule.owner from state (#69873)

- Remove AlertRule.owner from django state.
- Remove all usage of owner, and tighten the type checks for owners

Refs HC-1184

fixtures/backup/model_dependencies/detailed.json

@@ -361,11 +361,6 @@
         "model": "sentry.organization",
         "nullable": true
       },
-      "owner": {
-        "kind": "FlexibleForeignKey",
-        "model": "sentry.actor",
-        "nullable": true
-      },
       "snuba_query": {
         "kind": "FlexibleForeignKey",
         "model": "sentry.snubaquery",

fixtures/backup/model_dependencies/flat.json

@@ -49,7 +49,6 @@
     "sentry.user"
   ],
   "sentry.alertrule": [
-    "sentry.actor",
     "sentry.organization",
     "sentry.snubaquery",
     "sentry.team",

migrations_lockfile.txt

@@ -9,5 +9,5 @@ feedback: 0004_index_together
 hybridcloud: 0016_add_control_cacheversion
 nodestore: 0002_nodestore_no_dictfield
 replays: 0004_index_together
-sentry: 0708_rule_remove_owner_state
+sentry: 0709_alertrule_remove_owner_state
 social_auth: 0002_default_auto_field

src/sentry/backup/imports.py

@@ -68,6 +68,8 @@ DELETED_FIELDS: dict[
     "sentry.team": {"org_role"},
     # TODO(mark): Safe to remove after july 2024 after self-hosted 24.6.0 is released
     "sentry.rule": {"owner"},
+    # TODO(mark): Safe to remove after july 2024 after self-hosted 24.6.0 is released
+    "sentry.alertrule": {"owner"},
 }
 
 # The maximum number of models that may be sent at a time.

src/sentry/deletions/defaults/team.py

@@ -21,7 +21,7 @@ class TeamDeletionTask(ModelDeletionTask):
         from sentry.models.rule import Rule
         from sentry.monitors.models import Monitor
 
-        AlertRule.objects.filter(team_id=instance.id).update(owner=None, team_id=None)
+        AlertRule.objects.filter(team_id=instance.id).update(team_id=None)
         Rule.objects.filter(owner_team_id=instance.id).update(owner_team_id=None)
         Monitor.objects.filter(owner_team_id=instance.id).update(owner_team_id=None)
         super().delete_instance(instance)

src/sentry/incidents/logic.py

@@ -44,10 +44,11 @@ from sentry.incidents.models.incident import (
     IncidentTrigger,
     TriggerStatus,
 )
-from sentry.models.actor import Actor
 from sentry.models.notificationaction import ActionService, ActionTarget
 from sentry.models.project import Project
 from sentry.models.scheduledeletion import RegionScheduledDeletion
+from sentry.models.team import Team
+from sentry.models.user import User
 from sentry.relay.config.metric_extraction import on_demand_metrics_feature_flags
 from sentry.search.events.builder import QueryBuilder
 from sentry.search.events.fields import is_function, resolve_field
@@ -80,6 +81,7 @@ from sentry.snuba.subscriptions import (
 from sentry.snuba.tasks import build_query_builder
 from sentry.tasks.relay import schedule_invalidate_project_config
 from sentry.utils import metrics
+from sentry.utils.actor import ActorTuple
 from sentry.utils.audit import create_audit_entry_from_user
 from sentry.utils.snuba import is_measurement
 
@@ -503,7 +505,7 @@ def create_alert_rule(
     time_window,
     threshold_type,
     threshold_period,
-    owner=None,
+    owner: ActorTuple | None = None,
     resolve_threshold=None,
     environment=None,
     include_all_projects=False,
@@ -525,7 +527,7 @@ def create_alert_rule(
     if `include_all_projects` is True
     :param name: Name for the alert rule. This will be used as part of the
     incident name, and must be unique per project
-    :param owner: ActorTuple (sentry.models.actor.ActorTuple) or None
+    :param owner: ActorTuple (sentry.utils.actor.ActorTuple) or None
     :param query: An event search query to subscribe to and monitor for alerts
     :param aggregate: A string representing the aggregate used in this alert rule
     :param time_window: Time period to aggregate over, in minutes
@@ -557,12 +559,16 @@ def create_alert_rule(
         resolution = resolution * DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER
         comparison_delta = int(timedelta(minutes=comparison_delta).total_seconds())
 
-    # TODO(mark) type is documented as ActorTuple but these runtime checks are for other types.
-    actor = None
-    if owner and not isinstance(owner, Actor):
-        actor = owner.resolve_to_actor()
-    elif owner and isinstance(owner, Actor):
-        actor = owner
+    owner_user_id = None
+    owner_team_id = None
+    if owner and isinstance(owner, ActorTuple):
+        if owner.type == User:
+            owner_user_id = owner.id
+        elif owner.type == Team:
+            owner_team_id = owner.id
+    elif owner:
+        # TODO(mark) Remove this once it has been verified to not happen in CI
+        assert False, "Cannot create, invalid input type for owner"
 
     with transaction.atomic(router.db_for_write(SnubaQuery)):
         # NOTE: `create_snuba_query` constructs the postgres representation of the snuba query
@@ -585,11 +591,9 @@ def create_alert_rule(
             resolve_threshold=resolve_threshold,
             threshold_period=threshold_period,
             include_all_projects=include_all_projects,
-            # TODO(mark) remove owner in the future
-            owner=actor,
             comparison_delta=comparison_delta,
-            user_id=actor.user_id if actor else None,
-            team_id=actor.team_id if actor else None,
+            user_id=owner_user_id,
+            team_id=owner_team_id,
             monitor_type=monitor_type.value,
         )
 
@@ -654,9 +658,9 @@ def snapshot_alert_rule(alert_rule, user=None):
         alert_rule_snapshot.id = None
         alert_rule_snapshot.status = AlertRuleStatus.SNAPSHOT.value
         alert_rule_snapshot.snuba_query = snuba_query_snapshot
-        if alert_rule.owner:
-            alert_rule_snapshot.user_id = alert_rule.owner.user_id
-            alert_rule_snapshot.team_id = alert_rule.owner.team_id
+        if alert_rule.user_id or alert_rule.team_id:
+            alert_rule_snapshot.user_id = alert_rule.user_id
+            alert_rule_snapshot.team_id = alert_rule.team_id
         alert_rule_snapshot.save()
         AlertRuleActivity.objects.create(
             alert_rule=alert_rule_snapshot,
@@ -689,7 +693,7 @@ def update_alert_rule(
     dataset=None,
     projects=None,
     name=None,
-    owner=NOT_SET,
+    owner: ActorTuple | None | object = NOT_SET,
     query=None,
     aggregate=None,
     time_window=None,
@@ -713,7 +717,7 @@ def update_alert_rule(
     `include_all_projects` is True
     :param name: Name for the alert rule. This will be used as part of the
     incident name, and must be unique per project.
-    :param owner: ActorTuple (sentry.models.actor.ActorTuple) or None
+    :param owner: ActorTuple (sentry.utils.actor.ActorTuple) or None
     :param query: An event search query to subscribe to and monitor for alerts
     :param aggregate: A string representing the aggregate used in this alert rule
     :param time_window: Time period to aggregate over, in minutes.
@@ -761,11 +765,18 @@ def update_alert_rule(
     if event_types is not None:
         updated_query_fields["event_types"] = event_types
     if owner is not NOT_SET:
-        if owner is not None and not isinstance(owner, Actor):
-            owner = owner.resolve_to_actor()
-        updated_fields["owner"] = owner
-        updated_fields["team_id"] = owner.team_id if owner else None
-        updated_fields["user_id"] = owner.user_id if owner else None
+        team_id = None
+        user_id = None
+        if owner and isinstance(owner, ActorTuple):
+            if owner.type == User:
+                user_id = owner.id
+            elif owner.type == Team:
+                team_id = owner.id
+        elif owner:
+            # TODO(mark) Remove this once it has been verified to not happen in CI
+            assert False, "Cannot update, invalid input type for owner"
+        updated_fields["team_id"] = team_id
+        updated_fields["user_id"] = user_id
     if comparison_delta is not NOT_SET:
         if comparison_delta is not None:
             # Since comparison alerts make twice as many queries, run the queries less frequently.

src/sentry/incidents/models/alert_rule.py

@@ -14,9 +14,7 @@ from django.db.models import Q, QuerySet
 from django.db.models.signals import post_delete, post_save
 from django.utils import timezone
 
-from sentry.backup.dependencies import PrimaryKeyMap
-from sentry.backup.helpers import ImportFlags
-from sentry.backup.scopes import ImportScope, RelocationScope
+from sentry.backup.scopes import RelocationScope
 from sentry.constants import ObjectStatus
 from sentry.db.models import (
     BoundedPositiveIntegerField,
@@ -32,7 +30,6 @@ from sentry.incidents.models.alert_rule_activations import AlertRuleActivations
 from sentry.incidents.models.incident import IncidentTrigger
 from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE
 from sentry.incidents.utils.types import AlertRuleActivationConditionType
-from sentry.models.actor import Actor
 from sentry.models.notificationaction import AbstractNotificationAction, ActionService, ActionTarget
 from sentry.models.project import Project
 from sentry.models.team import Team
@@ -252,12 +249,7 @@ class AlertRule(Model):
         "sentry.Project", related_name="alert_rule_projects", through=AlertRuleProjects
     )
     snuba_query = FlexibleForeignKey("sentry.SnubaQuery", null=True, unique=True)
-    # Deprecated use user_id or team_id instead.
-    owner = FlexibleForeignKey(
-        "sentry.Actor",
-        null=True,
-        on_delete=models.SET_NULL,
-    )
+
     user_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete="SET_NULL")
     team = FlexibleForeignKey("sentry.Team", null=True, on_delete=models.SET_NULL)
 
@@ -290,15 +282,6 @@ class AlertRule(Model):
 
     __repr__ = sane_repr("id", "name", "date_added")
 
-    def _validate_actor(self):
-        # TODO(mark): Remove once owner is fully removed.
-        if self.owner_id is not None and self.team_id is None and self.user_id is None:
-            raise ValueError("AlertRule with owner requires either team_id or user_id")
-
-    def save(self, *args, **kwargs: Any) -> None:
-        self._validate_actor()
-        return super().save(*args, **kwargs)
-
     @property
     def created_by_id(self):
         try:
@@ -313,31 +296,6 @@ class AlertRule(Model):
     def get_audit_log_data(self):
         return {"label": self.name}
 
-    def normalize_before_relocation_import(
-        self, pk_map: PrimaryKeyMap, scope: ImportScope, flags: ImportFlags
-    ) -> int | None:
-        old_pk = super().normalize_before_relocation_import(pk_map, scope, flags)
-        if old_pk is None:
-            return None
-
-        # TODO(hybrid-cloud): actor refactor. Remove this check once we're sure we've migrated all
-        # remaining `owner_id`'s to also have `team_id` or `user_id`, which seems to not be the case
-        # today.
-        if self.owner_id is not None and self.team_id is None and self.user_id is None:
-            actor = Actor.objects.filter(id=self.owner_id).first()
-            if actor is None or (actor.team_id is None and actor.user_id is None):
-                # The `owner_id` references a non-existent `Actor`, or else one that has no
-                # `team_id` or `user_id` of its own, making it functionally a null `Actor`. This
-                # means the `owner_id` is invalid, so we simply delete it.
-                self.owner_id = None
-            else:
-                # Looks like an existing `Actor` points to a valid team or user - make sure that
-                # information is duplicated into this `AlertRule` model as well.
-                self.team_id = actor.team_id
-                self.user_id = actor.user_id
-
-        return old_pk
-
     def subscribe_projects(
         self,
         projects: list[Project],

src/sentry/incidents/serializers/alert_rule.py

@@ -87,12 +87,10 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
         allow_null=True,
     )
     aggregate = serializers.CharField(required=True, min_length=1)
-    # TODO(mark) This needs special handling to set user_id and team_id
-    owner = ActorField(
-        required=False,
-        allow_null=True,
-        as_actor=True,
-    )  # This will be set to required=True once the frontend starts sending it.
+
+    # This will be set to required=True once the frontend starts sending it.
+    owner = ActorField(required=False, allow_null=True)
+
     monitor_type = serializers.IntegerField(required=False, min_value=0)
     activation_condition = serializers.IntegerField(required=False, allow_null=True, min_value=0)
 
@@ -131,13 +129,6 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
         AlertRuleThresholdType.BELOW: lambda threshold: 100 - threshold,
     }
 
-    def validate_owner(self, owner):
-        # owner should be team:id or user:id
-        if owner is None:
-            return
-
-        return owner
-
     def validate_query(self, query):
         query_terms = query.split()
         for query_term in query_terms:

src/sentry/migrations/0709_alertrule_remove_owner_state.py

@@ -0,0 +1,53 @@
+# Generated by Django 5.0.4 on 2024-04-29 16:29
+
+from django.db import migrations
+
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production.
+    # This should only be used for operations where it's safe to run the migration after your
+    # code has deployed. So this should not be used for most operations that alter the schema
+    # of a table.
+    # Here are some things that make sense to mark as post deployment:
+    # - Large data migrations. Typically we want these to be run manually so that they can be
+    #   monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   run this outside deployments so that we don't block them. Note that while adding an index
+    #   is a schema change, it's completely safe to run the operation after the code has deployed.
+    # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
+
+    is_post_deployment = False
+
+    dependencies = [
+        ("sentry", "0708_rule_remove_owner_state"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            database_operations=[
+                migrations.RunSQL(
+                    sql="""
+                    ALTER TABLE "sentry_alertrule" DROP CONSTRAINT IF EXISTS "sentry_alertrule_owner_id_477ec831_fk_sentry_actor_id"
+                    """,
+                    reverse_sql="",
+                    hints={"tables": ["sentry_alertrule"]},
+                ),
+                # Follow up from 0708
+                migrations.RunSQL(
+                    sql="""
+                    ALTER TABLE "sentry_rule" DROP CONSTRAINT IF EXISTS "sentry_rule_owner_id_aa4e908b_fk_sentry_actor_id"
+                    """,
+                    reverse_sql="",
+                    hints={"tables": ["sentry_rule"]},
+                ),
+            ],
+            state_operations=[
+                migrations.RemoveField(
+                    model_name="alertrule",
+                    name="owner",
+                ),
+            ],
+        )
+    ]

src/sentry/models/project.py

@@ -502,7 +502,7 @@ class Project(Model, PendingDeletionMixin, OptionMixin, SnowflakeIdMixin):
                     organization_id=organization.id, id=rule.team_id
                 ).exists()
             if not is_member:
-                rule.update(team_id=None, user_id=None, owner=None)
+                rule.update(team_id=None, user_id=None)
         rules = Rule.objects.filter(
             Q(owner_team_id__isnull=False) | Q(owner_user_id__isnull=False), project=self
         )
@@ -577,9 +577,7 @@ class Project(Model, PendingDeletionMixin, OptionMixin, SnowflakeIdMixin):
         from sentry.models.rule import Rule
 
         ProjectTeam.objects.filter(project=self, team=team).delete()
-        AlertRule.objects.fetch_for_project(self).filter(team_id=team.id).update(
-            owner=None, team_id=None
-        )
+        AlertRule.objects.fetch_for_project(self).filter(team_id=team.id).update(team_id=None)
         Rule.objects.filter(owner_team_id=team.id, project=self).update(owner_team_id=None)
 
     def get_security_token(self):