diff --git a/ee/clickhouse/models/test/test_filters.py b/ee/clickhouse/models/test/test_filters.py index 26ff79c565c4e..def3ca4493fe1 100644 --- a/ee/clickhouse/models/test/test_filters.py +++ b/ee/clickhouse/models/test/test_filters.py @@ -610,6 +610,42 @@ def test_numerical(self): events = _filter_events(filter, self.team) self.assertEqual(events[0]["id"], event1_uuid) + def test_numerical_person_properties(self): + _create_person(team_id=self.team.pk, distinct_ids=["p1"], properties={"$a_number": 4}) + _create_person(team_id=self.team.pk, distinct_ids=["p2"], properties={"$a_number": 5}) + _create_person(team_id=self.team.pk, distinct_ids=["p3"], properties={"$a_number": 6}) + + filter = Filter( + data={ + "properties": [ + { + "type": "person", + "key": "$a_number", + "value": 4, + "operator": "gt", + } + ] + } + ) + self.assertEqual(len(_filter_persons(filter, self.team)), 2) + + filter = Filter(data={"properties": [{"type": "person", "key": "$a_number", "value": 5}]}) + self.assertEqual(len(_filter_persons(filter, self.team)), 1) + + filter = Filter( + data={ + "properties": [ + { + "type": "person", + "key": "$a_number", + "value": 6, + "operator": "lt", + } + ] + } + ) + self.assertEqual(len(_filter_persons(filter, self.team)), 2) + def test_contains(self): _create_event(team=self.team, distinct_id="test", event="$pageview") event2_uuid = _create_event( diff --git a/posthog/models/feature_flag/flag_matching.py b/posthog/models/feature_flag/flag_matching.py index fe57b171204e8..df9a83df7b682 100644 --- a/posthog/models/feature_flag/flag_matching.py +++ b/posthog/models/feature_flag/flag_matching.py @@ -10,7 +10,7 @@ from django.db import DatabaseError, IntegrityError, OperationalError from django.db.models.expressions import ExpressionWrapper, RawSQL from django.db.models.fields import BooleanField -from django.db.models import Q +from django.db.models import Q, Func, F, CharField from django.db.models.query import QuerySet from sentry_sdk.api import capture_exception, start_span from posthog.metrics import LABEL_TEAM_ID @@ -396,6 +396,13 @@ def condition_eval(key, condition): annotate_query = True nonlocal person_query + property_list = Filter(data=condition).property_groups.flat + properties_with_math_operators = [ + key_and_field_for_property(prop) + for prop in property_list + if prop.operator in ["gt", "lt", "gte", "lte"] + ] + if len(condition.get("properties", {})) > 0: # Feature Flags don't support OR filtering yet target_properties = self.property_value_overrides @@ -404,8 +411,9 @@ def condition_eval(key, condition): self.cache.group_type_index_to_name[feature_flag.aggregation_group_type_index], {}, ) + expr = properties_to_Q( - Filter(data=condition).property_groups.flat, + property_list, override_property_values=target_properties, cohorts_cache=self.cohorts_cache, using_database=DATABASE_FOR_FLAG_MATCHING, @@ -428,13 +436,24 @@ def condition_eval(key, condition): if annotate_query: if feature_flag.aggregation_group_type_index is None: + # :TRICKY: Flag matching depends on type of property when doing >, <, >=, <= comparisons. + # This requires a generated field to query in Q objects, which sadly don't allow inlining fields, + # hence we need to annotate the query here, even though these annotations are used much deeper, + # in properties_to_q, in empty_or_null_with_value_q + # These need to come in before the expr so they're available to use inside the expr. + # Same holds for the group queries below. + type_property_annotations = { + prop_key: Func(F(prop_field), function="JSONB_TYPEOF", output_field=CharField()) + for prop_key, prop_field in properties_with_math_operators + } person_query = person_query.annotate( + **type_property_annotations, **{ key: ExpressionWrapper( expr if expr else RawSQL("true", []), output_field=BooleanField(), - ) - } + ), + }, ) person_fields.append(key) else: @@ -445,13 +464,18 @@ def condition_eval(key, condition): group_query, group_fields, ) = group_query_per_group_type_mapping[feature_flag.aggregation_group_type_index] + type_property_annotations = { + prop_key: Func(F(prop_field), function="JSONB_TYPEOF", output_field=CharField()) + for prop_key, prop_field in properties_with_math_operators + } group_query = group_query.annotate( + **type_property_annotations, **{ key: ExpressionWrapper( expr if expr else RawSQL("true", []), output_field=BooleanField(), ) - } + }, ) group_fields.append(key) group_query_per_group_type_mapping[feature_flag.aggregation_group_type_index] = ( @@ -881,3 +905,12 @@ def parse_exception_for_error_message(err: Exception): reason = "query_wait_timeout" return reason + + +def key_and_field_for_property(property: Property) -> Tuple[str, str]: + column = "group_properties" if property.type == "group" else "properties" + key = property.key + return ( + f"{column}_{key}_type", + f"{column}__{key}", + ) diff --git a/posthog/models/filters/test/test_filter.py b/posthog/models/filters/test/test_filter.py index 84b70bbd4d837..72ad460b91084 100644 --- a/posthog/models/filters/test/test_filter.py +++ b/posthog/models/filters/test/test_filter.py @@ -2,7 +2,7 @@ import json from typing import Any, Callable, Dict, List, Optional, cast -from django.db.models import Q +from django.db.models import Q, Func, F, CharField from posthog.constants import FILTER_TEST_ACCOUNTS from posthog.models import Cohort, Filter, Person, Team @@ -219,42 +219,6 @@ def test_incomplete_data(self): ) self.assertListEqual(filter.property_groups.values, []) - def test_numerical_person_properties(self): - person_factory(team_id=self.team.pk, distinct_ids=["p1"], properties={"$a_number": 4}) - person_factory(team_id=self.team.pk, distinct_ids=["p2"], properties={"$a_number": 5}) - person_factory(team_id=self.team.pk, distinct_ids=["p3"], properties={"$a_number": 6}) - - filter = Filter( - data={ - "properties": [ - { - "type": "person", - "key": "$a_number", - "value": 4, - "operator": "gt", - } - ] - } - ) - self.assertEqual(len(filter_persons(filter, self.team)), 2) - - filter = Filter(data={"properties": [{"type": "person", "key": "$a_number", "value": 5}]}) - self.assertEqual(len(filter_persons(filter, self.team)), 1) - - filter = Filter( - data={ - "properties": [ - { - "type": "person", - "key": "$a_number", - "value": 6, - "operator": "lt", - } - ] - } - ) - self.assertEqual(len(filter_persons(filter, self.team)), 2) - def test_contains_persons(self): person_factory( team_id=self.team.pk, @@ -819,6 +783,56 @@ def _filter_with_date_range( return Filter(data=data) + def test_numerical_person_properties(self): + _create_person(team_id=self.team.pk, distinct_ids=["p1"], properties={"$a_number": 4}) + _create_person(team_id=self.team.pk, distinct_ids=["p2"], properties={"$a_number": 5}) + _create_person(team_id=self.team.pk, distinct_ids=["p3"], properties={"$a_number": 6}) + _create_person(team_id=self.team.pk, distinct_ids=["p4"], properties={"$a_number": 14}) + + flush_persons_and_events() + + def filter_persons_with_annotation(filter: Filter, team: Team): + persons = Person.objects.annotate( + **{ + "properties_$a_number_type": Func( + F("properties__$a_number"), function="JSONB_TYPEOF", output_field=CharField() + ) + } + ).filter(properties_to_Q(filter.property_groups.flat)) + persons = persons.filter(team_id=team.pk) + return [str(uuid) for uuid in persons.values_list("uuid", flat=True)] + + filter = Filter( + data={ + "properties": [ + { + "type": "person", + "key": "$a_number", + "value": "4", + "operator": "gt", + } + ] + } + ) + self.assertEqual(len(filter_persons_with_annotation(filter, self.team)), 3) + + filter = Filter(data={"properties": [{"type": "person", "key": "$a_number", "value": 5}]}) + self.assertEqual(len(filter_persons_with_annotation(filter, self.team)), 1) + + filter = Filter( + data={ + "properties": [ + { + "type": "person", + "key": "$a_number", + "value": 6, + "operator": "lt", + } + ] + } + ) + self.assertEqual(len(filter_persons_with_annotation(filter, self.team)), 2) + def filter_persons_with_property_group( filter: Filter, team: Team, property_overrides: Dict[str, Any] = {} diff --git a/posthog/queries/base.py b/posthog/queries/base.py index 65ccd651d2c3c..00269a384b37f 100644 --- a/posthog/queries/base.py +++ b/posthog/queries/base.py @@ -12,7 +12,7 @@ ) from dateutil import parser -from django.db.models import Exists, OuterRef, Q +from django.db.models import Exists, OuterRef, Q, Value from rest_framework.exceptions import ValidationError from posthog.constants import PropertyOperatorType @@ -29,10 +29,10 @@ from posthog.queries.util import convert_to_datetime_aware from posthog.utils import get_compare_period_dates, is_valid_regex -F = TypeVar("F", Filter, PathFilter) +FilterType = TypeVar("FilterType", Filter, PathFilter) -def determine_compared_filter(filter: F) -> F: +def determine_compared_filter(filter: FilterType) -> FilterType: if not filter.date_to or not filter.date_from: raise ValidationError("You need date_from and date_to to compare") date_from, date_to = get_compare_period_dates( @@ -142,8 +142,34 @@ def compute_exact_match(value: ValueT, override_value: Any) -> bool: except re.error: return False - if operator == "gt": - return type(override_value) == type(value) and override_value > value + if operator in ("gt", "gte", "lt", "lte"): + # :TRICKY: We adjust comparison based on the override value passed in, + # to make sure we handle both numeric and string comparisons appropriately. + def compare(lhs, rhs, operator): + if operator == "gt": + return lhs > rhs + elif operator == "gte": + return lhs >= rhs + elif operator == "lt": + return lhs < rhs + elif operator == "lte": + return lhs <= rhs + else: + raise ValueError(f"Invalid operator: {operator}") + + parsed_value = None + try: + parsed_value = float(value) # type: ignore + except Exception: + pass + + if parsed_value is not None: + if isinstance(override_value, str): + return compare(override_value, str(value), operator) + else: + return compare(override_value, parsed_value, operator) + else: + return compare(str(override_value), str(value), operator) if operator == "gte": return type(override_value) == type(value) and override_value >= value @@ -207,7 +233,25 @@ def empty_or_null_with_value_q( f"{column}__{key}", value_as_coerced_to_number ) else: - target_filter = Q(**{f"{column}__{key}__{operator}": value}) + if isinstance(value, list): + raise TypeError(f"empty_or_null_with_value_q: Operator {operator} does not support list values") + + parsed_value = None + if operator in ("gt", "gte", "lt", "lte"): + try: + parsed_value = float(value) + except (ValueError, TypeError): + pass + + if parsed_value is not None: + # When we can coerce given value to a number, check whether the value in DB is a number + # and do a numeric comparison. Otherwise, do a string comparison. + target_filter = Q( + Q(**{f"{column}__{key}__{operator}": str(value), f"{column}_{key}_type": Value("string")}) + | Q(**{f"{column}__{key}__{operator}": parsed_value, f"{column}_{key}_type": Value("number")}) + ) + else: + target_filter = Q(**{f"{column}__{key}__{operator}": value}) query_filter = Q(target_filter & Q(**{f"{column}__has_key": key}) & ~Q(**{f"{column}__{key}": None})) diff --git a/posthog/queries/test/test_base.py b/posthog/queries/test/test_base.py index bccc9ca60a53e..91076d31aa4ff 100644 --- a/posthog/queries/test/test_base.py +++ b/posthog/queries/test/test_base.py @@ -154,7 +154,8 @@ def test_match_properties_math_operators(self): self.assertFalse(match_property(property_a, {"key": 0})) self.assertFalse(match_property(property_a, {"key": -1})) - self.assertFalse(match_property(property_a, {"key": "23"})) + # now we handle type mismatches so this should be true + self.assertTrue(match_property(property_a, {"key": "23"})) property_b = Property(key="key", value=1, operator="lt") self.assertTrue(match_property(property_b, {"key": 0})) @@ -171,16 +172,32 @@ def test_match_properties_math_operators(self): self.assertFalse(match_property(property_c, {"key": 0})) self.assertFalse(match_property(property_c, {"key": -1})) - self.assertFalse(match_property(property_c, {"key": "3"})) + # now we handle type mismatches so this should be true + self.assertTrue(match_property(property_c, {"key": "3"})) property_d = Property(key="key", value="43", operator="lt") self.assertTrue(match_property(property_d, {"key": "41"})) self.assertTrue(match_property(property_d, {"key": "42"})) + self.assertTrue(match_property(property_d, {"key": 42})) self.assertFalse(match_property(property_d, {"key": "43"})) self.assertFalse(match_property(property_d, {"key": "44"})) self.assertFalse(match_property(property_d, {"key": 44})) + property_e = Property(key="key", value="30", operator="lt") + self.assertTrue(match_property(property_e, {"key": "29"})) + + # depending on the type of override, we adjust type comparison + self.assertTrue(match_property(property_e, {"key": "100"})) + self.assertFalse(match_property(property_e, {"key": 100})) + + property_f = Property(key="key", value="123aloha", operator="gt") + self.assertFalse(match_property(property_f, {"key": "123"})) + self.assertFalse(match_property(property_f, {"key": 122})) + + # this turns into a string comparison + self.assertTrue(match_property(property_f, {"key": 129})) + def test_match_property_date_operators(self): property_a = Property(key="key", value="2022-05-01", operator="is_date_before") self.assertTrue(match_property(property_a, {"key": "2022-03-01"})) diff --git a/posthog/test/__snapshots__/test_feature_flag.ambr b/posthog/test/__snapshots__/test_feature_flag.ambr index f7a912fe972b7..a4200a05ffd09 100644 --- a/posthog/test/__snapshots__/test_feature_flag.ambr +++ b/posthog/test/__snapshots__/test_feature_flag.ambr @@ -345,6 +345,58 @@ AND "posthog_group"."group_type_index" = 2) ' --- +# name: TestFeatureFlagMatcher.test_numeric_operator_with_groups_and_person_flags + ' + SELECT "posthog_grouptypemapping"."id", + "posthog_grouptypemapping"."team_id", + "posthog_grouptypemapping"."group_type", + "posthog_grouptypemapping"."group_type_index", + "posthog_grouptypemapping"."name_singular", + "posthog_grouptypemapping"."name_plural" + FROM "posthog_grouptypemapping" + WHERE "posthog_grouptypemapping"."team_id" = 2 + ' +--- +# name: TestFeatureFlagMatcher.test_numeric_operator_with_groups_and_person_flags.1 + ' + SELECT (((("posthog_person"."properties" -> 'number') >= '"20"' + AND JSONB_TYPEOF(("posthog_person"."properties" -> 'number')) = 'string') + OR (("posthog_person"."properties" -> 'number') >= '20.0' + AND JSONB_TYPEOF(("posthog_person"."properties" -> 'number')) = 'number')) + AND "posthog_person"."properties" ? 'number' + AND NOT (("posthog_person"."properties" -> 'number') = 'null')) AS "flag_X_condition_0" + FROM "posthog_person" + INNER JOIN "posthog_persondistinctid" ON ("posthog_person"."id" = "posthog_persondistinctid"."person_id") + WHERE ("posthog_persondistinctid"."distinct_id" = '307' + AND "posthog_persondistinctid"."team_id" = 2 + AND "posthog_person"."team_id" = 2) + ' +--- +# name: TestFeatureFlagMatcher.test_numeric_operator_with_groups_and_person_flags.2 + ' + SELECT (((("posthog_group"."group_properties" -> 'number') > '"100"' + AND JSONB_TYPEOF(("posthog_group"."group_properties" -> 'number')) = 'string') + OR (("posthog_group"."group_properties" -> 'number') > '100.0' + AND JSONB_TYPEOF(("posthog_group"."group_properties" -> 'number')) = 'number')) + AND "posthog_group"."group_properties" ? 'number' + AND NOT (("posthog_group"."group_properties" -> 'number') = 'null')) AS "flag_X_condition_0" + FROM "posthog_group" + WHERE ("posthog_group"."team_id" = 2 + AND "posthog_group"."group_key" = 'foo' + AND "posthog_group"."group_type_index" = 0) + ' +--- +# name: TestFeatureFlagMatcher.test_numeric_operator_with_groups_and_person_flags.3 + ' + SELECT (("posthog_group"."group_properties" -> 'number') > '"100b2c"' + AND "posthog_group"."group_properties" ? 'number' + AND NOT (("posthog_group"."group_properties" -> 'number') = 'null')) AS "flag_X_condition_0" + FROM "posthog_group" + WHERE ("posthog_group"."team_id" = 2 + AND "posthog_group"."group_key" = 'foo-project' + AND "posthog_group"."group_type_index" = 1) + ' +--- # name: TestFeatureFlagMatcher.test_super_condition_matches_string ' SELECT ((("posthog_person"."properties" -> 'is_enabled') = 'true' diff --git a/posthog/test/base.py b/posthog/test/base.py index 105285fa3073e..3cd830a68a49d 100644 --- a/posthog/test/base.py +++ b/posthog/test/base.py @@ -460,13 +460,14 @@ def assertQueryMatchesSnapshot(self, query, params=None, replace_all_numbers=Fal if replace_all_numbers: query = re.sub(r"(\"?) = \d+", r"\1 = 2", query) query = re.sub(r"(\"?) IN \(\d+(, \d+)*\)", r"\1 IN (1, 2, 3, 4, 5 /* ... */)", query) - # feature flag conditions use primary keys as columns in queries, so replace those too - query = re.sub(r"flag_\d+_condition", r"flag_X_condition", query) - query = re.sub(r"flag_\d+_super_condition", r"flag_X_super_condition", query) else: query = re.sub(r"(team|cohort)_id(\"?) = \d+", r"\1_id\2 = 2", query) query = re.sub(r"\d+ as (team|cohort)_id(\"?)", r"2 as \1_id\2", query) + # feature flag conditions use primary keys as columns in queries, so replace those always + query = re.sub(r"flag_\d+_condition", r"flag_X_condition", query) + query = re.sub(r"flag_\d+_super_condition", r"flag_X_super_condition", query) + # hog ql checks team ids differently query = re.sub( r"equals\(([^.]+\.)?team_id?, \d+\)", diff --git a/posthog/test/test_feature_flag.py b/posthog/test/test_feature_flag.py index dbcda391f1f22..83bfe2583a482 100644 --- a/posthog/test/test_feature_flag.py +++ b/posthog/test/test_feature_flag.py @@ -4139,6 +4139,449 @@ def create_groups(self): def create_feature_flag(self, key="beta-feature", **kwargs): return FeatureFlag.objects.create(team=self.team, name="Beta feature", key=key, created_by=self.user, **kwargs) + def test_numeric_operator(self): + Person.objects.create( + team=self.team, + distinct_ids=["307"], + properties={"number": 30, "string_number": "30", "version": "1.24"}, + ) + + feature_flag1 = self.create_feature_flag( + key="random1", + filters={ + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "100", + "operator": "gt", + "type": "person", + }, + ] + } + ] + }, + ) + + feature_flag2 = self.create_feature_flag( + key="random2", + filters={ + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "100b2c", + "operator": "gt", + "type": "person", + }, + ] + } + ] + }, + ) + + feature_flag3 = self.create_feature_flag( + key="random3", + filters={ + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "3.1x00b2c", + "operator": "gt", + "type": "person", + }, + ] + } + ] + }, + ) + + feature_flag4 = self.create_feature_flag( + key="random4", + filters={ + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "20", + "operator": "gt", + "type": "person", + }, + ] + } + ] + }, + ) + + feature_flag5 = self.create_feature_flag( + key="random5", + filters={ + "groups": [ + { + "properties": [ + { + "key": "version", + "value": "1.05", + "operator": "gt", + "type": "person", + }, + ] + }, + { + "properties": [ + { + "key": "version", + "value": "1.15", + "operator": "gt", + "type": "person", + }, + ] + }, + { + "properties": [ + { + "key": "version", + "value": "1.1200", + "operator": "gt", + "type": "person", + }, + ] + }, + ] + }, + ) + + feature_flag6 = self.create_feature_flag( + key="random6", + filters={ + "groups": [ + { + "properties": [ + { + "key": "version", + "value": "1.206.0", + "operator": "lt", + "type": "person", + }, + ] + } + ] + }, + ) + + self.assertEqual( + self.match_flag(feature_flag1, "307"), + FeatureFlagMatch(False, None, FeatureFlagMatchReason.NO_CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag(feature_flag2, "307"), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag(feature_flag3, "307"), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag(feature_flag4, "307"), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + + # even though we can parse as a number, only do string comparison + self.assertEqual( + self.match_flag(feature_flag5, "307"), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag(feature_flag6, "307"), + FeatureFlagMatch(False, None, FeatureFlagMatchReason.NO_CONDITION_MATCH, 0), + ) + + def test_numeric_operator_with_groups_and_person_flags(self): + Person.objects.create( + team=self.team, + distinct_ids=["307"], + properties={"number": 30, "string_number": "30", "version": "1.24"}, + ) + GroupTypeMapping.objects.create(team=self.team, group_type="organization", group_type_index=0) + GroupTypeMapping.objects.create(team=self.team, group_type="project", group_type_index=1) + + Group.objects.create( + team=self.team, + group_type_index=0, + group_key="foo", + group_properties={"name": "foo.inc", "number": 50, "string_number": "50"}, + version=1, + ) + Group.objects.create( + team=self.team, + group_type_index=1, + group_key="foo-project", + group_properties={"name": "foo-project", "number": 20, "string_number": "20"}, + version=1, + ) + + feature_flag1 = self.create_feature_flag( + key="random1", + filters={ + "aggregation_group_type_index": 0, + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "100", + "operator": "gt", + "group_type_index": 0, + "type": "group", + }, + ] + } + ], + }, + ) + + feature_flag2 = self.create_feature_flag( + key="random2", + filters={ + "aggregation_group_type_index": 1, + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "100b2c", + "operator": "gt", + "group_type_index": 1, + "type": "group", + }, + ] + } + ], + }, + ) + + feature_flag3 = self.create_feature_flag( + key="random3", + filters={ + "aggregation_group_type_index": 0, + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "3.1x00b2c", + "operator": "gte", + "type": "person", + "group_type_index": 0, + "type": "group", + }, + ] + } + ], + }, + ) + + feature_flag4 = self.create_feature_flag( + key="random4", + filters={ + "aggregation_group_type_index": 0, + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "20", + "operator": "gt", + "group_type_index": 0, + "type": "group", + }, + ] + } + ], + }, + ) + + feature_flag4_person = self.create_feature_flag( + key="random4_person", + filters={ + "groups": [ + { + "properties": [ + { + "key": "number", + "value": "20", + "operator": "gte", + "type": "person", + }, + ] + } + ] + }, + ) + + feature_flag5 = self.create_feature_flag( + key="random5", + filters={ + "groups": [ + { + "properties": [ + { + "key": "version", + "value": "1.05", + "operator": "gt", + "type": "person", + }, + ] + }, + { + "properties": [ + { + "key": "version", + "value": "1.15", + "operator": "gt", + "type": "person", + }, + ] + }, + { + "properties": [ + { + "key": "version", + "value": "1.1200", + "operator": "gte", + "type": "person", + }, + ] + }, + ] + }, + ) + + feature_flag6 = self.create_feature_flag( + key="random6", + filters={ + "aggregation_group_type_index": 0, + "groups": [ + { + "properties": [ + { + "key": "version", + "value": "1.206.0", + "operator": "lt", + "group_type_index": 0, + "type": "group", + }, + ] + } + ], + }, + ) + + self.assertEqual( + self.match_flag(feature_flag1, "307", groups={"organization": "foo", "project": "foo-project"}), + FeatureFlagMatch(False, None, FeatureFlagMatchReason.NO_CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag(feature_flag2, "307", groups={"organization": "foo", "project": "foo-project"}), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag(feature_flag3, "307", groups={"organization": "foo", "project": "foo-project"}), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag(feature_flag4, "307", groups={"organization": "foo", "project": "foo-project"}), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + + # even though we can parse as a number, only do string comparison + self.assertEqual( + self.match_flag(feature_flag5, "307", groups={"organization": "foo", "project": "foo-project"}), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag(feature_flag6, "307", groups={"organization": "foo", "project": "foo-project"}), + FeatureFlagMatch(False, None, FeatureFlagMatchReason.NO_CONDITION_MATCH, 0), + ) + + # Make sure clashes on property name doesn't affect computation + with snapshot_postgres_queries_context(self, replace_all_numbers=False): + self.assertEqual( + FeatureFlagMatcher( + [feature_flag1, feature_flag2, feature_flag4_person], + "307", + groups={"organization": "foo", "project": "foo-project"}, + ).get_matches()[1], + { + "random1": { + "condition_index": 0, + "reason": FeatureFlagMatchReason.NO_CONDITION_MATCH, + }, + "random2": { + "condition_index": 0, + "reason": FeatureFlagMatchReason.CONDITION_MATCH, + }, + "random4_person": { + "condition_index": 0, + "reason": FeatureFlagMatchReason.CONDITION_MATCH, + }, + }, + ) + + # handle overrides in group properties + self.assertEqual( + self.match_flag( + feature_flag1, + "307", + groups={"organization": "foo", "project": "foo-project"}, + group_property_value_overrides={"organization": {"number": 200}, "project": {"number": 1}}, + ), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + + # string '30' > string '100' (lexicographically) + self.assertEqual( + self.match_flag( + feature_flag1, + "307", + groups={"organization": "foo", "project": "foo-project"}, + group_property_value_overrides={"organization": {"number": "30"}, "project": {"number": 1}}, + ), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag( + feature_flag1, + "307", + groups={"organization": "foo", "project": "foo-project"}, + group_property_value_overrides={"organization": {"number": "01323"}, "project": {"number": 1}}, + ), + FeatureFlagMatch(False, None, FeatureFlagMatchReason.NO_CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag( + feature_flag1, + "307", + groups={"organization": "foo", "project": "foo-project"}, + group_property_value_overrides={"organization": {"number": 0}, "project": {"number": 1}}, + ), + FeatureFlagMatch(False, None, FeatureFlagMatchReason.NO_CONDITION_MATCH, 0), + ) + self.assertEqual( + self.match_flag( + feature_flag2, + "307", + groups={"organization": "foo", "project": "foo-project"}, + group_property_value_overrides={"organization": {"number": "0"}, "project": {"number": 19.999999}}, + ), + FeatureFlagMatch(True, None, FeatureFlagMatchReason.CONDITION_MATCH, 0), + ) + class TestFeatureFlagHashKeyOverrides(BaseTest, QueryMatchingTest): person: Person