feat(workflow_engine): Refactor the Data Condition Validator (#88050)

## Description
- Separate the DataCondition and DataConditionGroup validators
- Add tests to the BaseDataConditionGroupValidator
- Add tests for BaseDataConditionValidator
- Create `AbstractDataConditionValidator` to easily allow definitions,
this meant we could completely remove the numeric validator as we can
just use the abstract validator.

---------

Co-authored-by: Cathy Teng <[email protected]>

Author: 2 people

src/sentry/incidents/endpoints/validators/__init__.py

@@ -4,7 +4,6 @@
 from rest_framework import serializers
 
 from sentry import audit_log
-from sentry.incidents.endpoints.validators import NumericComparisonConditionValidator
 from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
 from sentry.snuba.snuba_query_validator import SnubaQueryValidator
 from sentry.snuba.subscriptions import update_snuba_query
@@ -13,6 +12,9 @@
     BaseDataConditionGroupValidator,
     BaseDetectorTypeValidator,
 )
+from sentry.workflow_engine.endpoints.validators.base.data_condition import (
+    AbstractDataConditionValidator,
+)
 from sentry.workflow_engine.models import DataConditionGroup, DataSource, Detector
 from sentry.workflow_engine.models.data_condition import Condition, DataCondition
 from sentry.workflow_engine.types import (
@@ -22,17 +24,53 @@
 )
 
 
-class MetricAlertComparisonConditionValidator(NumericComparisonConditionValidator):
+class MetricAlertComparisonConditionValidator(
+    AbstractDataConditionValidator[float, DetectorPriorityLevel]
+):
     supported_conditions = frozenset((Condition.GREATER, Condition.LESS))
     supported_condition_results = frozenset(
         (DetectorPriorityLevel.HIGH, DetectorPriorityLevel.MEDIUM)
     )
-    condition_group_id = serializers.IntegerField(required=True)
-    id = serializers.IntegerField(required=False)
+
+    def validate_type(self, value: str) -> Condition:
+        try:
+            type = Condition(value)
+        except ValueError:
+            type = None
+
+        if type not in self.supported_conditions:
+            raise serializers.ValidationError(f"Unsupported type {value}")
+
+        return type
+
+    def validate_comparison(self, value: float | int | str) -> float:
+        try:
+            value = float(value)
+        except ValueError:
+            raise serializers.ValidationError("A valid number is required.")
+
+        return value
+
+    def validate_condition_result(self, value: str) -> DetectorPriorityLevel:
+        try:
+            result = DetectorPriorityLevel(int(value))
+        except ValueError:
+            result = None
+
+        if result not in self.supported_condition_results:
+            raise serializers.ValidationError("Unsupported condition result")
+
+        return result
 
 
 class MetricAlertConditionGroupValidator(BaseDataConditionGroupValidator):
-    conditions = MetricAlertComparisonConditionValidator(many=True)
+    conditions = serializers.ListField(required=True)
+
+    def validate_conditions(self, value):
+        MetricAlertComparisonConditionValidator(data=value, many=True).is_valid(
+            raise_exception=True
+        )
+        return value
 
 
 class MetricAlertsDetectorValidator(BaseDetectorTypeValidator):

src/sentry/incidents/endpoints/validators/numeric_comparison_validator.py

@@ -1,4 +1,5 @@
 __all__ = [
+    "AbstractDataConditionValidator",
     "BaseActionValidator",
     "BaseDataConditionGroupValidator",
     "BaseDataConditionValidator",
@@ -8,6 +9,7 @@
 ]
 
 from .action import BaseActionValidator
-from .data_condition import BaseDataConditionGroupValidator, BaseDataConditionValidator
+from .data_condition import AbstractDataConditionValidator, BaseDataConditionValidator
+from .data_condition_group import BaseDataConditionGroupValidator
 from .data_source import BaseDataSourceValidator, DataSourceCreator
 from .detector import BaseDetectorTypeValidator

src/sentry/incidents/metric_alert_detector.py

@@ -1,12 +1,10 @@
 from typing import Any, Generic, TypeVar
 
-from django.forms import ValidationError
-from jsonschema import ValidationError as JsonValidationError
-from jsonschema import validate
 from rest_framework import serializers
 
 from sentry.api.serializers.rest_framework import CamelSnakeModelSerializer
 from sentry.db.models import Model
+from sentry.workflow_engine.endpoints.validators.utils import validate_json_schema
 from sentry.workflow_engine.models import Action
 from sentry.workflow_engine.registry import action_handler_registry
 from sentry.workflow_engine.types import ActionHandler
@@ -16,39 +14,19 @@
 ActionConfig = dict[str, Any]
 
 
-def validate_json_schema(value, schema):
-    try:
-        validate(value, schema)
-    except JsonValidationError as e:
-        raise ValidationError(str(e))
-
-    return value
-
-
 class BaseActionValidator(CamelSnakeModelSerializer[T], Generic[T]):
     data: Any = serializers.JSONField()
     config: Any = serializers.JSONField()
+    type = serializers.ChoiceField(choices=[(t.value, t.name) for t in Action.Type])
+    integration_id = serializers.IntegerField()
 
     class Meta:
         model = T
-        fields = ["config", "data", "integration_id", "type"]
 
     def _get_action_handler(self) -> ActionHandler:
-        initial_type = self.initial_data.get("type")
-        action_type = self.validate_type(initial_type)
+        action_type = self.initial_data.get("type")
         return action_handler_registry.get(action_type)
 
-    def validate_type(self, value) -> Action.Type:
-        if not value:
-            raise ValidationError("Action type is required")
-
-        try:
-            Action.Type(value)
-        except ValueError:
-            raise ValidationError("Invalid action type")
-
-        return value
-
     def validate_data(self, value) -> ActionData:
         data_schema = self._get_action_handler().data_schema
         return validate_json_schema(value, data_schema)

src/sentry/workflow_engine/endpoints/validators/base/__init__.py

@@ -1,30 +1,77 @@
+from abc import abstractmethod
+from typing import Any, Generic, TypeVar
+
+from jsonschema import ValidationError as JsonValidationError
 from rest_framework import serializers
-from rest_framework.fields import Field
 
 from sentry.api.serializers.rest_framework import CamelSnakeSerializer
+from sentry.utils.registry import NoRegistrationExistsError
+from sentry.workflow_engine.endpoints.validators.utils import validate_json_schema
+from sentry.workflow_engine.models.data_condition import CONDITION_OPS, Condition
+from sentry.workflow_engine.registry import condition_handler_registry
+from sentry.workflow_engine.types import DataConditionHandler
+
+ComparisonType = TypeVar("ComparisonType")
+ConditionResult = TypeVar("ConditionResult")
+
+
+class AbstractDataConditionValidator(
+    CamelSnakeSerializer,
+    Generic[ComparisonType, ConditionResult],
+):
+    id = serializers.IntegerField(required=False)
+    type = serializers.ChoiceField(choices=[(t.value, t.value) for t in Condition])
+    comparison = serializers.JSONField(required=True)
+    condition_result = serializers.JSONField(required=True)
+    condition_group_id = serializers.IntegerField(required=True)
+
+    @abstractmethod
+    def validate_comparison(self, value: Any) -> ComparisonType:
+        pass
+
+    @abstractmethod
+    def validate_condition_result(self, value: Any) -> ConditionResult:
+        pass
+
 
+class BaseDataConditionValidator(
+    AbstractDataConditionValidator[Any, Any],
+):
+    def _get_handler(self) -> DataConditionHandler | None:
+        condition_type = self.initial_data.get("type")
+        if condition_type in CONDITION_OPS:
+            return None
 
-class BaseDataConditionValidator(CamelSnakeSerializer):
-    type = serializers.CharField(
-        required=True,
-        max_length=200,
-        help_text="Condition used to compare data value to the stored comparison value",
-    )
+        try:
+            return condition_handler_registry.get(condition_type)
+        except NoRegistrationExistsError:
+            raise serializers.ValidationError(f"Invalid condition type: {condition_type}")
 
-    @property
-    def comparison(self) -> Field:
-        raise NotImplementedError
+    def validate_comparison(self, value: Any) -> Any:
+        handler = self._get_handler()
 
-    @property
-    def result(self) -> Field:
-        raise NotImplementedError
+        if not handler:
+            raise serializers.ValidationError(
+                "Condition Operators should implement their own validators for comparison"
+            )
 
-    def validate(self, attrs):
-        attrs = super().validate(attrs)
-        return attrs
+        try:
+            return validate_json_schema(value, handler.comparison_json_schema)
+        except JsonValidationError:
+            raise serializers.ValidationError(
+                f"Value, {value} does not match JSON Schema for comparison"
+            )
 
+    def validate_condition_result(self, value: Any) -> Any:
+        handler = self._get_handler()
+        if not handler:
+            raise serializers.ValidationError(
+                "Condition Operators should implement their own validation for condition_result"
+            )
 
-class BaseDataConditionGroupValidator(CamelSnakeSerializer):
-    logic_type = serializers.CharField(required=True)
-    organization_id = serializers.IntegerField(required=True)
-    conditions = BaseDataConditionValidator(many=True)
+        try:
+            return validate_json_schema(value, handler.condition_result_schema)
+        except JsonValidationError:
+            raise serializers.ValidationError(
+                f"Value, {value}, does not match JSON Schema for condition result"
+            )

src/sentry/workflow_engine/endpoints/validators/base/action.py

@@ -0,0 +1,10 @@
+from rest_framework import serializers
+
+from sentry.api.serializers.rest_framework import CamelSnakeSerializer
+from sentry.workflow_engine.models import DataConditionGroup
+
+
+class BaseDataConditionGroupValidator(CamelSnakeSerializer):
+    logic_type = serializers.ChoiceField([(t.value, t.value) for t in DataConditionGroup.Type])
+    organization_id = serializers.IntegerField(required=True)
+    conditions = serializers.ListField(required=False)

src/sentry/workflow_engine/endpoints/validators/base/data_condition.py

@@ -0,0 +1,12 @@
+from django.forms import ValidationError
+from jsonschema import ValidationError as JsonValidationError
+from jsonschema import validate
+
+
+def validate_json_schema(value, schema):
+    try:
+        validate(value, schema)
+    except JsonValidationError as e:
+        raise ValidationError(str(e))
+
+    return value

src/sentry/workflow_engine/endpoints/validators/base/data_condition_group.py

@@ -4,6 +4,7 @@
     "ActionGroupStatus",
     "AlertRuleDetector",
     "AlertRuleWorkflow",
+    "Condition",
     "DataCondition",
     "DataConditionGroup",
     "DataConditionGroupAction",
@@ -23,7 +24,7 @@
 from .action_group_status import ActionGroupStatus
 from .alertrule_detector import AlertRuleDetector
 from .alertrule_workflow import AlertRuleWorkflow
-from .data_condition import DataCondition
+from .data_condition import Condition, DataCondition
 from .data_condition_group import DataConditionGroup
 from .data_condition_group_action import DataConditionGroupAction
 from .data_source import DataPacket, DataSource

src/sentry/workflow_engine/endpoints/validators/utils.py

@@ -84,6 +84,7 @@ class Subgroup(StrEnum):
     group: ClassVar[Group]
     subgroup: ClassVar[Subgroup]
     comparison_json_schema: ClassVar[dict[str, Any]] = {}
+    condition_result_schema: ClassVar[dict[str, Any]] = {}
 
     @staticmethod
     def evaluate_value(value: T, comparison: Any) -> DataConditionResult: