diff --git a/src/sentry/workflow_engine/endpoints/validators/base/data_condition_group.py b/src/sentry/workflow_engine/endpoints/validators/base/data_condition_group.py index bd79711c474a..3119f74717a3 100644 --- a/src/sentry/workflow_engine/endpoints/validators/base/data_condition_group.py +++ b/src/sentry/workflow_engine/endpoints/validators/base/data_condition_group.py @@ -10,7 +10,12 @@ ) from sentry.workflow_engine.endpoints.validators.utils import remove_items_by_api_input from sentry.workflow_engine.models import DataConditionGroup -from sentry.workflow_engine.models.data_condition import TRIGGER_CONDITIONS, DataCondition +from sentry.workflow_engine.models.data_condition import ( + Condition, + DataCondition, + get_condition_handler, +) +from sentry.workflow_engine.types import DataConditionHandler class DataConditionGroupInput(TypedDict): @@ -40,9 +45,19 @@ def _validate_logic_type(self, condition_data: list[dict[str, Any]], logic_type: break upon updating. """ for condition in condition_data: - if (condition.get("type") in TRIGGER_CONDITIONS) and ( - logic_type != DataConditionGroup.Type.ANY_SHORT_CIRCUIT.value - ): + try: + condition_type = Condition(str(condition.get("type"))) + except ValueError: + raise serializers.ValidationError( + f"Invalid condition type, '{condition.get('type')}'" + ) + + condition_handler = get_condition_handler(condition_type) + + if ( + condition_handler is not None + and condition_handler.group == DataConditionHandler.Group.WORKFLOW_TRIGGER + ) and (logic_type != DataConditionGroup.Type.ANY_SHORT_CIRCUIT.value): raise serializers.ValidationError("Triggers' logic type must be 'any-short'") def update_or_create_condition( diff --git a/src/sentry/workflow_engine/models/data_condition.py b/src/sentry/workflow_engine/models/data_condition.py index c8e06e4b7cbe..e243396ac830 100644 --- a/src/sentry/workflow_engine/models/data_condition.py +++ b/src/sentry/workflow_engine/models/data_condition.py @@ -14,7 +14,12 @@ DataConditionEvaluationException, ) from sentry.workflow_engine.registry import condition_handler_registry -from sentry.workflow_engine.types import ConditionError, DataConditionResult, DetectorPriorityLevel +from sentry.workflow_engine.types import ( + ConditionError, + DataConditionHandler, + DataConditionResult, + DetectorPriorityLevel, +) from sentry.workflow_engine.utils import scopedstats logger = logging.getLogger(__name__) @@ -70,13 +75,6 @@ class Condition(StrEnum): SEER_ACTIVITY_TRIGGER = "seer_activity_trigger" -TRIGGER_CONDITIONS = [ - Condition.FIRST_SEEN_EVENT, - Condition.ISSUE_RESOLVED_TRIGGER, - Condition.REAPPEARED_EVENT, - Condition.REGRESSION_EVENT, -] - CONDITION_OPS = { Condition.EQUAL: operator.eq, Condition.GREATER_OR_EQUAL: operator.ge, @@ -260,6 +258,18 @@ def evaluate_value(self, value: T) -> DataConditionEvaluation: ) +def get_condition_handler( + condition_type: Condition, +) -> type[DataConditionHandler[Any]] | None: + if condition_type not in CONDITION_OPS: + try: + return condition_handler_registry.get(condition_type) + except registry.NoRegistrationExistsError: + pass + + return None + + def is_slow_condition(condition: DataCondition) -> bool: return Condition(condition.type) in SLOW_CONDITIONS diff --git a/tests/sentry/workflow_engine/endpoints/validators/test_base_data_condition_group.py b/tests/sentry/workflow_engine/endpoints/validators/test_base_data_condition_group.py index 20c5f5b4765b..cde4c44afffb 100644 --- a/tests/sentry/workflow_engine/endpoints/validators/test_base_data_condition_group.py +++ b/tests/sentry/workflow_engine/endpoints/validators/test_base_data_condition_group.py @@ -149,6 +149,44 @@ def test_create__with_conditions(self) -> None: assert condition.comparison == 1 assert condition.condition_group == result + def test_create_operator_condition__non_short_circuit_logic_type(self) -> None: + self.valid_data["logicType"] = DataConditionGroup.Type.ALL + self.valid_data["conditions"] = [ + { + "type": Condition.EQUAL, + "comparison": 1, + "conditionResult": True, + } + ] + + validator = BaseDataConditionGroupValidator(data=self.valid_data, context=self.context) + validator.is_valid(raise_exception=True) + result = validator.create(validator.validated_data) + + assert result.logic_type == DataConditionGroup.Type.ALL + assert result.conditions.count() == 1 + + def test_create_action_filter__non_short_circuit_logic_type(self) -> None: + self.valid_data["logicType"] = DataConditionGroup.Type.ALL + self.valid_data["conditions"] = [ + { + "type": Condition.AGE_COMPARISON, + "comparison": { + "comparison_type": "older", + "value": 1, + "time": "day", + }, + "conditionResult": True, + } + ] + + validator = BaseDataConditionGroupValidator(data=self.valid_data, context=self.context) + validator.is_valid(raise_exception=True) + result = validator.create(validator.validated_data) + + assert result.logic_type == DataConditionGroup.Type.ALL + assert result.conditions.count() == 1 + def test_create_trigger__valid_logic_type(self) -> None: valid_data = { "organizationId": self.organization.id, @@ -194,7 +232,9 @@ def test_create_trigger__invalid_logic_type(self) -> None: "logicType": DataConditionGroup.Type.ALL, "conditions": [ { - "type": Condition.FIRST_SEEN_EVENT, + # This trigger was never included in the former TRIGGER_CONDITIONS list. + # Its handler metadata must be the source of truth for validation. + "type": Condition.EVERY_EVENT, "comparison": True, "conditionResult": True, } diff --git a/tests/sentry/workflow_engine/models/test_data_condition.py b/tests/sentry/workflow_engine/models/test_data_condition.py index 7d10ce5046d7..aaad3e59e8ed 100644 --- a/tests/sentry/workflow_engine/models/test_data_condition.py +++ b/tests/sentry/workflow_engine/models/test_data_condition.py @@ -1,10 +1,12 @@ from enum import IntEnum +from typing import Any, cast from unittest import mock import pytest from sentry.testutils.cases import TestCase -from sentry.workflow_engine.models.data_condition import Condition +from sentry.utils.registry import NoRegistrationExistsError +from sentry.workflow_engine.models.data_condition import Condition, get_condition_handler from sentry.workflow_engine.processors.evaluations import DataConditionEvaluationException from sentry.workflow_engine.types import ConditionError, DetectorPriorityLevel from tests.sentry.workflow_engine.test_base import BaseWorkflowTest, DataConditionHandlerMixin @@ -45,10 +47,26 @@ def test_boolean(self) -> None: assert dc.get_condition_result() is True +class GetConditionHandlerTest(TestCase): + def test_operator_condition(self) -> None: + assert get_condition_handler(Condition.EQUAL) is None + + def test_registered_condition(self) -> None: + assert get_condition_handler(Condition.EVERY_EVENT) is not None + + @mock.patch("sentry.workflow_engine.models.data_condition.condition_handler_registry.get") + def test_unregistered_condition(self, mock_get: mock.Mock) -> None: + mock_get.side_effect = NoRegistrationExistsError + + assert get_condition_handler(Condition.AGE_COMPARISON) is None + + class EvaluateValueTest(DataConditionHandlerMixin, BaseWorkflowTest): def test(self) -> None: dc = self.create_data_condition( - type=Condition.GREATER, comparison=1.0, condition_result=DetectorPriorityLevel.HIGH + type=Condition.GREATER, + comparison=cast(Any, 1.0), + condition_result=DetectorPriorityLevel.HIGH, ) evaluation = dc.evaluate_value(2) assert evaluation.triggered is True @@ -84,7 +102,9 @@ def test_bad_condition(self) -> None: with pytest.raises(ValueError): # Raises ValueError because the condition is invalid self.create_data_condition( - type="invalid", comparison=1.0, condition_result=DetectorPriorityLevel.HIGH + type=cast(Any, "invalid"), + comparison=cast(Any, 1.0), + condition_result=DetectorPriorityLevel.HIGH, ) def test_bad_comparison(self) -> None: @@ -100,7 +120,7 @@ def test_bad_comparison(self) -> None: def test_condition_result_comparison_fails(self) -> None: dc = self.create_data_condition( type=Condition.GREATER, - comparison=1.0, + comparison=cast(Any, 1.0), condition_result="wrong", ) evaluation = dc.evaluate_value(2)