chore(aci): enforce config schema without subclassing

src/sentry/incidents/grouptype.py

@@ -26,3 +26,4 @@ class MetricAlertFire(GroupType):
     enable_escalation_detection = False
     detector_handler = MetricAlertDetectorHandler
     detector_validator = MetricAlertsDetectorValidator
+    detector_config_schema = {}  # TODO(colleen): update this

src/sentry/issues/grouptype.py

@@ -16,7 +16,7 @@
 from sentry.features.base import OrganizationFeature
 from sentry.ratelimits.sliding_windows import Quota
 from sentry.types.group import PriorityLevel
-from sentry.utils import metrics
+from sentry.utils import json, metrics
 
 if TYPE_CHECKING:
     from sentry.models.organization import Organization
@@ -174,11 +174,20 @@ class GroupType:
     notification_config: NotificationConfig = NotificationConfig()
     detector_handler: type[DetectorHandler] | None = None
     detector_validator: type[BaseGroupTypeDetectorValidator] | None = None
+    detector_config_schema: dict[str, Any] | None = None
 
     def __init_subclass__(cls: type[GroupType], **kwargs: Any) -> None:
         super().__init_subclass__(**kwargs)
         registry.add(cls)
 
+        from sentry.workflow_engine.registry import detector_config_schema_registry
+
+        detector_config_schema = cls.detector_config_schema or {}
+        if (
+            cls.slug not in detector_config_schema_registry.registrations
+        ):  # TODO(cathy): remove after updating getsentry test with patch
+            detector_config_schema_registry.register(cls.slug)(json.dumps(detector_config_schema))
+
         if not cls.released:
             features.add(cls.build_visible_feature_name(), OrganizationFeature, True)
             features.add(cls.build_ingest_feature_name(), OrganizationFeature)
@@ -560,12 +569,14 @@ class MonitorIncidentType(GroupType):
 class MonitorCheckInTimeout(MonitorIncidentType):
     # This is deprecated, only kept around for it's type_id
     type_id = 4002
+    slug = "monitor_check_in_timeout"
 
 
 @dataclass(frozen=True)
 class MonitorCheckInMissed(MonitorIncidentType):
     # This is deprecated, only kept around for it's type_id
     type_id = 4003
+    slug = "monitor_check_in_missed"
 
 
 @dataclass(frozen=True)

src/sentry/testutils/fixtures.py

@@ -12,6 +12,7 @@
 from sentry.incidents.models.alert_rule import AlertRule, AlertRuleMonitorTypeInt
 from sentry.integrations.models.integration import Integration
 from sentry.integrations.models.organization_integration import OrganizationIntegration
+from sentry.issues.grouptype import ErrorGroupType
 from sentry.models.activity import Activity
 from sentry.models.environment import Environment
 from sentry.models.grouprelease import GroupRelease
@@ -635,12 +636,13 @@ def create_detector(
         self,
         *args,
         project=None,
+        type=ErrorGroupType.slug,
         **kwargs,
     ) -> Detector:
         if project is None:
             project = self.create_project(organization=self.organization)
 
-        return Factories.create_detector(*args, project=project, **kwargs)
+        return Factories.create_detector(*args, project=project, type=type, **kwargs)
 
     def create_detector_state(self, *args, **kwargs) -> DetectorState:
         return Factories.create_detector_state(*args, **kwargs)

src/sentry/utils/registry.py

@@ -15,9 +15,10 @@ class NoRegistrationExistsError(ValueError):
 
 
 class Registry(Generic[T]):
-    def __init__(self):
+    def __init__(self, enable_reverse_lookup: bool = True):
         self.registrations: dict[str, T] = {}
         self.reverse_lookup: dict[T, str] = {}
+        self.enable_reverse_lookup = enable_reverse_lookup
 
     def register(self, key: str):
         def inner(item: T) -> T:
@@ -26,13 +27,14 @@ def inner(item: T) -> T:
                     f"A registration already exists for {key}: {self.registrations[key]}"
                 )
 
-            if item in self.reverse_lookup:
-                raise AlreadyRegisteredError(
-                    f"A registration already exists for {item}: {self.reverse_lookup[item]}"
-                )
+            if self.enable_reverse_lookup:
+                if item in self.reverse_lookup:
+                    raise AlreadyRegisteredError(
+                        f"A registration already exists for {item}: {self.reverse_lookup[item]}"
+                    )
+                self.reverse_lookup[item] = key
 
             self.registrations[key] = item
-            self.reverse_lookup[item] = key
 
             return item
 
@@ -44,6 +46,8 @@ def get(self, key: str) -> T:
         return self.registrations[key]
 
     def get_key(self, item: T) -> str:
+        if not self.enable_reverse_lookup:
+            raise NoRegistrationExistsError("Reverse lookup is not enabled")
         if item not in self.reverse_lookup:
             raise NoRegistrationExistsError(f"No registration exists for {item}")
         return self.reverse_lookup[item]

src/sentry/workflow_engine/models/detector.py

@@ -7,13 +7,16 @@
 from django.conf import settings
 from django.db import models
 from django.db.models import UniqueConstraint
+from django.db.models.signals import pre_save
+from django.dispatch import receiver
 
 from sentry.backup.scopes import RelocationScope
 from sentry.db.models import DefaultFieldsModel, FlexibleForeignKey, region_silo_model
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
 from sentry.issues import grouptype
 from sentry.issues.grouptype import GroupType
 from sentry.models.owner_base import OwnerModel
+from sentry.utils import json
 
 from .json_config import JSONConfigBase
 
@@ -60,10 +63,6 @@ class Detector(DefaultFieldsModel, OwnerModel, JSONConfigBase):
     # The user that created the detector
     created_by_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete="SET_NULL")
 
-    @property
-    def CONFIG_SCHEMA(self) -> dict[str, Any]:
-        raise NotImplementedError('Subclasses must define a "CONFIG_SCHEMA" attribute')
-
     class Meta(OwnerModel.Meta):
         constraints = OwnerModel.Meta.constraints + [
             UniqueConstraint(
@@ -83,7 +82,6 @@ def detector_handler(self) -> DetectorHandler | None:
             logger.error(
                 "No registered grouptype for detector",
                 extra={
-                    "group_type": str(group_type),
                     "detector_id": self.id,
                     "detector_type": self.type,
                 },
@@ -105,3 +103,11 @@ def detector_handler(self) -> DetectorHandler | None:
     def get_audit_log_data(self) -> dict[str, Any]:
         # TODO: Create proper audit log data for the detector, group and conditions
         return {}
+
+
+@receiver(pre_save, sender=Detector)
+def enforce_config_schema(sender, instance: Detector, **kwargs):
+    from sentry.workflow_engine.registry import detector_config_schema_registry
+
+    config_schema = detector_config_schema_registry.get(instance.type)
+    instance.validate_config(json.loads(config_schema))

src/sentry/workflow_engine/models/json_config.py

@@ -1,4 +1,3 @@
-from abc import abstractproperty
 from typing import Any
 
 from django.db import models
@@ -8,13 +7,9 @@
 class JSONConfigBase(models.Model):
     config = models.JSONField(db_default={})
 
-    @abstractproperty
-    def CONFIG_SCHEMA(self) -> dict[str, Any]:
-        pass
-
-    def validate_config(self) -> None:
+    def validate_config(self, schema: dict[str, Any]) -> None:
         try:
-            validate(self.config, self.CONFIG_SCHEMA)
+            validate(self.config, schema)
         except ValidationError as e:
             raise ValidationError(f"Invalid config: {e.message}")
 

src/sentry/workflow_engine/models/workflow.py

@@ -2,6 +2,8 @@
 
 from django.conf import settings
 from django.db import models
+from django.db.models.signals import pre_save
+from django.dispatch import receiver
 
 from sentry.backup.scopes import RelocationScope
 from sentry.db.models import DefaultFieldsModel, FlexibleForeignKey, region_silo_model, sane_repr
@@ -36,7 +38,8 @@ class Workflow(DefaultFieldsModel, OwnerModel, JSONConfigBase):
 
     @property
     def CONFIG_SCHEMA(self) -> dict[str, Any]:
-        raise NotImplementedError('Subclasses must define a "CONFIG_SCHEMA" attribute')
+        # TODO: fill in
+        return {}
 
     __repr__ = sane_repr("name", "organization_id")
 
@@ -60,3 +63,9 @@ def evaluate_trigger_conditions(self, evt: GroupEvent) -> bool:
 
         evaluation, _ = evaluate_condition_group(self.when_condition_group, evt)
         return evaluation
+
+
+@receiver(pre_save, sender=Workflow)
+def enforce_config_schema(sender, instance: Workflow, **kwargs):
+    config_schema = instance.CONFIG_SCHEMA
+    instance.validate_config(config_schema)

src/sentry/workflow_engine/processors/detector.py

@@ -3,11 +3,12 @@
 import logging
 
 from sentry.eventstore.models import GroupEvent
+from sentry.issues.grouptype import ErrorGroupType
 from sentry.issues.issue_occurrence import IssueOccurrence
 from sentry.issues.producer import PayloadType, produce_occurrence_to_kafka
 from sentry.workflow_engine.handlers.detector import DetectorEvaluationResult
 from sentry.workflow_engine.models import DataPacket, Detector
-from sentry.workflow_engine.types import DetectorGroupKey, DetectorType
+from sentry.workflow_engine.types import DetectorGroupKey
 
 logger = logging.getLogger(__name__)
 
@@ -17,7 +18,7 @@ def get_detector_by_event(evt: GroupEvent) -> Detector:
     issue_occurrence = evt.occurrence
 
     if issue_occurrence is None:
-        detector = Detector.objects.get(project_id=evt.project_id, type=DetectorType.ERROR)
+        detector = Detector.objects.get(project_id=evt.project_id, type=ErrorGroupType.slug)
     else:
         detector = Detector.objects.get(id=issue_occurrence.evidence_data.get("detector_id", None))
 

src/sentry/workflow_engine/registry.py

@@ -6,3 +6,7 @@
 data_source_type_registry = Registry[type[DataSourceTypeHandler]]()
 condition_handler_registry = Registry[DataConditionHandler[Any]]()
 action_handler_registry = Registry[ActionHandler]()
+
+detector_config_schema_registry = Registry[str](
+    enable_reverse_lookup=False
+)  # json dump, allow duplicate values

tests/sentry/issues/test_grouptype.py

@@ -17,6 +17,7 @@
     get_group_types_by_category,
 )
 from sentry.testutils.cases import TestCase
+from sentry.utils.registry import Registry
 
 
 class BaseGroupTypeTest(TestCase):
@@ -25,9 +26,16 @@ def setUp(self) -> None:
         self.registry_patcher = patch("sentry.issues.grouptype.registry", new=GroupTypeRegistry())
         self.registry_patcher.__enter__()
 
+        self.detector_registry_patcher = patch(
+            "sentry.workflow_engine.registry.detector_config_schema_registry",
+            new=Registry[str](enable_reverse_lookup=False),
+        )
+        self.detector_registry_patcher.__enter__()
+
     def tearDown(self) -> None:
         super().tearDown()
         self.registry_patcher.__exit__(None, None, None)
+        self.detector_registry_patcher.__exit__(None, None, None)
 
 
 class GroupTypeTest(BaseGroupTypeTest):

tests/sentry/workflow_engine/models/test_json_config_base.py

@@ -0,0 +1,83 @@
+from dataclasses import dataclass
+from unittest.mock import PropertyMock, patch
+
+import pytest
+from jsonschema import ValidationError
+
+from sentry.issues.grouptype import GroupCategory, GroupType
+from sentry.utils.registry import NoRegistrationExistsError
+from tests.sentry.issues.test_grouptype import BaseGroupTypeTest
+
+
+class TestJsonConfigBase(BaseGroupTypeTest):
+    def setUp(self):
+        super().setUp()
+        self.correct_config = {
+            "username": "user123",
+            "email": "[email protected]",
+            "fullName": "John Doe",
+            "age": 30,
+            "location": "Cityville",
+            "interests": ["Travel", "Technology"],
+        }
+
+        @dataclass(frozen=True)
+        class TestGroupType(GroupType):
+            type_id = 1
+            slug = "test"
+            description = "Test"
+            category = GroupCategory.ERROR.value
+            detector_config_schema = self.example_schema
+
+    @pytest.fixture(autouse=True)
+    def initialize_configs(self):
+        self.example_schema = {
+            "$id": "https://example.com/user-profile.schema.json",
+            "$schema": "https://json-schema.org/draft/2020-12/schema",
+            "description": "A representation of a user profile",
+            "type": "object",
+            "required": ["username", "email"],
+            "properties": {
+                "username": {"type": "string"},
+                "email": {"type": "string", "format": "email"},
+                "fullName": {"type": "string"},
+                "age": {"type": "integer", "minimum": 0},
+                "location": {"type": "string"},
+                "interests": {"type": "array", "items": {"type": "string"}},
+            },
+        }
+        with (
+            patch(
+                "sentry.workflow_engine.models.Workflow.CONFIG_SCHEMA",
+                return_value=self.example_schema,
+                new_callable=PropertyMock,
+            ),
+        ):
+            # Run test case
+            yield
+
+
+class TestDetectorConfig(TestJsonConfigBase):
+    def test_detector_no_registration(self):
+        with pytest.raises(NoRegistrationExistsError):
+            self.create_detector(name="test_detector", type="no_registration")
+
+    def test_detector_mismatched_schema(self):
+        with pytest.raises(ValidationError):
+            self.create_detector(name="test_detector", type="test", config={"hi": "there"})
+
+    def test_detector_correct_schema(self):
+        self.create_detector(name="test_detector", type="test", config=self.correct_config)
+
+
+class TestWorkflowConfig(TestJsonConfigBase):
+    def test_workflow_mismatched_schema(self):
+        with pytest.raises(ValidationError):
+            self.create_workflow(
+                organization=self.organization, name="test_workflow", config={"hi": "there"}
+            )
+
+    def test_workflow_correct_schema(self):
+        self.create_workflow(
+            organization=self.organization, name="test_workflow", config=self.correct_config
+        )

tests/sentry/workflow_engine/models/test_workflow.py

@@ -1,11 +1,10 @@
-from sentry.workflow_engine.types import DetectorType
 from tests.sentry.workflow_engine.test_base import BaseWorkflowTest
 
 
 class WorkflowTest(BaseWorkflowTest):
     def setUp(self):
         self.workflow, self.detector, self.detector_workflow, self.data_condition_group = (
-            self.create_detector_and_workflow(detector_type=DetectorType.ERROR)
+            self.create_detector_and_workflow()
         )
         self.data_condition = self.data_condition_group.conditions.first()
         self.group, self.event, self.group_event = self.create_group_event()

tests/sentry/workflow_engine/processors/test_detector.py

@@ -121,9 +121,16 @@ def test_state_results_multi_group(self, mock_produce_occurrence_to_kafka):
         )
 
     def test_no_issue_type(self):
-        detector = self.create_detector(type="invalid slug")
+        detector = self.create_detector(type=self.handler_state_type.slug)
         data_packet = self.build_data_packet()
-        with mock.patch("sentry.workflow_engine.models.detector.logger") as mock_logger:
+        with (
+            mock.patch("sentry.workflow_engine.models.detector.logger") as mock_logger,
+            mock.patch(
+                "sentry.workflow_engine.models.Detector.group_type",
+                return_value=None,
+                new_callable=mock.PropertyMock,
+            ),
+        ):
             results = process_detectors(data_packet, [detector])
             assert mock_logger.error.call_args[0][0] == "No registered grouptype for detector"
         assert results == []
@@ -326,7 +333,7 @@ def test_above_below_threshold(self):
         }
 
     def test_no_condition_group(self):
-        detector = self.create_detector()
+        detector = self.create_detector(type=self.handler_type.slug)
         handler = MockDetectorStateHandler(detector)
         with mock.patch(
             "sentry.workflow_engine.handlers.detector.stateful.metrics"