diff --git a/api/edge_api/identities/models.py b/api/edge_api/identities/models.py index bf36e0f3eca4..77864d48d1f7 100644 --- a/api/edge_api/identities/models.py +++ b/api/edge_api/identities/models.py @@ -4,7 +4,6 @@ from datetime import timedelta from django.conf import settings -from django.db.models import Prefetch, Q from django.utils import timezone from api_keys.user import APIKeyUser @@ -17,9 +16,6 @@ from edge_api.identities.utils import generate_change_dict from environments.dynamodb import DynamoIdentityWrapper from environments.models import Environment -from features.models import FeatureState -from features.multivariate.models import MultivariateFeatureStateValue -from features.versioning.versioning_service import get_environment_flags_dict from users.models import FFAdminUser from util.engine_models.features.models import FeatureStateModel from util.engine_models.identities.models import IdentityFeaturesList, IdentityModel @@ -77,70 +73,6 @@ def dashboard_alias(self, dashboard_alias: str) -> None: def add_feature_override(self, feature_state: FeatureStateModel) -> None: self.engine_identity_model.identity_features.append(feature_state) - def get_all_feature_states( - self, - ) -> typing.Tuple[ - typing.List[typing.Union[FeatureState, FeatureStateModel]], typing.Set[str] - ]: - """ - Get all feature states for a flag engine identity model. The list returned by - this function contains two distinct types: features.models.FeatureState & - flag_engine.features.models.FeatureStateModel. - - :return: tuple of (list of feature states, set of feature names that were overridden - for the identity specifically) - """ - segment_ids = self.dynamo_wrapper.get_segment_ids( - identity_model=self.engine_identity_model - ) - django_environment = self.environment - - # since identity overrides are included in the document retrieved from dynamo, - # we only want to retrieve the environment default and (relevant) segment overrides - # from the ORM. - additional_filters = Q(identity__isnull=True) & ( - Q(feature_segment__segment__id__in=segment_ids) - | Q(feature_segment__isnull=True) - ) - - feature_states: dict[str, FeatureState | FeatureStateModel] = ( - get_environment_flags_dict( # type: ignore[assignment] - environment=django_environment, - additional_filters=additional_filters, - additional_select_related_args=[ - "feature", - "feature_segment", - "feature_segment__segment", - "feature_state_value", - ], - additional_prefetch_related_args=[ - Prefetch( - "multivariate_feature_state_values", - queryset=MultivariateFeatureStateValue.objects.select_related( - "multivariate_feature_option" - ), - ) - ], - # since we only want to retrieve the highest priority feature state, - # we key off the feature name instead of the default - # (feature_id, segment_id, identity_id). This will give us only e.g. - # the highest priority matching segment override for a given feature. - key_function=lambda fs: fs.feature.name, # type: ignore[arg-type,return-value] - ) - ) - - # Since the identity overrides are the highest priority, we can now iterate - # over the dictionary and replace any feature states with those that have - # an identity override, stored against the identity in dynamo. - identity_feature_states = self.feature_overrides - identity_feature_names = set() - for identity_feature_state in identity_feature_states: - feature_name = identity_feature_state.feature.name - feature_states[feature_name] = identity_feature_state - identity_feature_names.add(feature_name) - - return list(feature_states.values()), identity_feature_names - def get_feature_state_by_feature_name_or_id( self, feature: typing.Union[str, int] ) -> typing.Optional[FeatureStateModel]: diff --git a/api/edge_api/identities/views.py b/api/edge_api/identities/views.py index f3812d037b59..34265cdec002 100644 --- a/api/edge_api/identities/views.py +++ b/api/edge_api/identities/views.py @@ -51,6 +51,7 @@ ) from environments.models import Environment from environments.permissions.permissions import NestedEnvironmentPermissions +from evaluation.services import get_edge_identity_feature_states from features.models import FeatureState from features.permissions import IdentityFeatureStatePermissions from projects.exceptions import DynamoNotEnabledError @@ -280,19 +281,13 @@ def perform_destroy(self, instance): # type: ignore[no-untyped-def] @extend_schema(responses={200: IdentityAllFeatureStatesSerializer(many=True)}) @action(detail=False, methods=["GET"]) def all(self, request, *args, **kwargs): # type: ignore[no-untyped-def] - ( - feature_states, - identity_feature_names, - ) = self.identity.get_all_feature_states() - serializer = IdentityAllFeatureStatesSerializer( - instance=feature_states, + instance=get_edge_identity_feature_states(self.identity), many=True, context={ "request": request, "identity": self.identity, "environment_api_key": self.identity.environment_api_key, - "identity_feature_names": identity_feature_names, }, ) diff --git a/api/environments/dynamodb/wrappers/identity_wrapper.py b/api/environments/dynamodb/wrappers/identity_wrapper.py index b649192b6d39..69cd36571555 100644 --- a/api/environments/dynamodb/wrappers/identity_wrapper.py +++ b/api/environments/dynamodb/wrappers/identity_wrapper.py @@ -22,10 +22,6 @@ from environments.identities.traits.constants import ( TRAIT_STRING_VALUE_MAX_LENGTH, ) -from util.engine_models.context.mappers import ( - is_context_in_segment, - map_environment_identity_to_context, -) from util.engine_models.identities.models import IdentityModel from util.mappers import ( map_engine_identity_to_identity_document, @@ -320,35 +316,3 @@ def search_items( query_kwargs.update(ExclusiveStartKey=start_key) return self.query_items(**query_kwargs) - - def get_segment_ids( - self, - identity_pk: str = None, # type: ignore[assignment] - identity_model: IdentityModel = None, # type: ignore[assignment] - ) -> list: # type: ignore[type-arg] - from environments.models import Environment - from util.mappers.engine import map_segment_to_engine - - if not (identity_pk or identity_model): - raise ValueError("Must provide one of identity_pk or identity_model.") - - with suppress(ObjectDoesNotExist): - identity = identity_model or IdentityModel.model_validate( - self.get_item_from_uuid(identity_pk) - ) - environment = Environment.objects.select_related("project").get( - api_key=identity.environment_api_key, - ) - segments = environment.project.get_segments_from_cache() - context = map_environment_identity_to_context( - environment=environment, - identity=identity, - override_traits=None, - ) - return [ - segment.id - for segment in segments - if is_context_in_segment(context, map_segment_to_engine(segment)) - ] - - return [] diff --git a/api/environments/identities/models.py b/api/environments/identities/models.py index 003a0bbca5f8..74bcbc893a3d 100644 --- a/api/environments/identities/models.py +++ b/api/environments/identities/models.py @@ -1,18 +1,15 @@ from itertools import chain from django.db import models -from django.db.models import Prefetch, Q from flag_engine.engine import get_evaluation_result from environments.identities.managers import IdentityManager from environments.identities.traits.models import Trait from environments.models import Environment from environments.sdk.types import SDKTraitData +from evaluation.mappers import map_environment_to_evaluation_context from features.models import FeatureState -from features.multivariate.models import MultivariateFeatureStateValue -from features.versioning.versioning_service import get_environment_flags_list from segments.models import Segment -from util.mappers.engine import map_environment_to_evaluation_context class Identity(models.Model): @@ -52,81 +49,6 @@ def get_hash_key(self, use_identity_composite_key_for_hashing: bool = False) -> else str(self.id) ) - def get_all_feature_states( - self, - feature_name: str | None = None, - traits: list[Trait] | None = None, - additional_filters: Q | None = None, - ) -> list[FeatureState]: - """ - Get all feature states for an identity. This method returns a single flag for - each feature in the identity's environment's project. The flag returned is the - correct flag based on the priorities as follows (highest -> lowest): - - 1. Identity - flag override for this specific identity - 2. Segment - flag overridden for a segment this identity belongs to - 3. Environment - default value for the environment - - :return: (list) flags for an identity with the correct values based on - identity / segment priorities - """ - segments = self.get_segments(traits=traits, overrides_only=True) - - # define sub queries - belongs_to_environment_query = Q(environment=self.environment) - if self.id: - overridden_for_identity_query = Q(identity=self) - else: - # skip identity overrides for transient identities - overridden_for_identity_query = Q() - - overridden_for_segment_query = Q( - feature_segment__segment__in=segments, - feature_segment__environment=self.environment, - ) - environment_default_query = Q(identity=None, feature_segment=None) - - # define the full query - full_query = belongs_to_environment_query & ( - overridden_for_identity_query - | overridden_for_segment_query - | environment_default_query - ) - - if additional_filters: - full_query &= additional_filters - - all_flags = get_environment_flags_list( - environment=self.environment, - feature_name=feature_name, - additional_filters=full_query, - additional_prefetch_related_args=[ - Prefetch( - "multivariate_feature_state_values", - queryset=MultivariateFeatureStateValue.objects.select_related( - "multivariate_feature_option" - ), - ) - ], - ) - - # iterate over all the flags and build a dictionary keyed on feature with the highest priority flag - # for the given identity as the value. - identity_flags = {} - for flag in all_flags: - if flag.feature_id not in identity_flags: - identity_flags[flag.feature_id] = flag - else: - current_flag = identity_flags[flag.feature_id] - if flag > current_flag: - identity_flags[flag.feature_id] = flag - - if self.environment.get_hide_disabled_flags() is True: - # filter out any flags that are disabled - return [value for value in identity_flags.values() if value.enabled] - - return list(identity_flags.values()) - def get_overridden_feature_states(self) -> dict[int, FeatureState]: """ Get all overridden feature states for an identity. @@ -166,8 +88,11 @@ def get_segments( ) result = get_evaluation_result(context) return [ - segments_by_pk[segment_result["metadata"]["pk"]] + segments_by_pk[pk] for segment_result in result["segments"] + # Synthetic identity-override segments carry no pk, and are not + # segments as far as any caller is concerned. + if (pk := segment_result["metadata"].get("pk")) is not None ] def get_all_user_traits(self): # type: ignore[no-untyped-def] diff --git a/api/environments/identities/serializers.py b/api/environments/identities/serializers.py index f3e47f42824f..2d66caf7b1d8 100644 --- a/api/environments/identities/serializers.py +++ b/api/environments/identities/serializers.py @@ -96,26 +96,27 @@ class IdentityAllFeatureStatesSerializer(serializers.Serializer): # type: ignor def get_feature_state_value( self, instance: typing.Union[FeatureState, FeatureStateModel] ) -> typing.Union[str, int, bool]: - identity = self.context["identity"] - environment_api_key = self.context["environment_api_key"] + if isinstance(instance, FeatureState): + return instance.evaluated_value # type: ignore[no-any-return] - environment = Environment.get_from_cache(environment_api_key) + # An edge identity's own overrides are stored in DynamoDB rather than + # the ORM, and are still resolved outside the engine. + environment = Environment.get_from_cache(self.context["environment_api_key"]) assert environment - hash_key = identity.get_hash_key( - environment.use_identity_composite_key_for_hashing + return instance.get_value( # type: ignore[no-any-return] + self.context["identity"].get_hash_key( + environment.use_identity_composite_key_for_hashing + ) ) - if isinstance(instance, FeatureState): - return instance.get_feature_state_value_by_hash_key(hash_key) # type: ignore[no-any-return] - - return instance.get_value(hash_key) # type: ignore[no-any-return] - def get_overridden_by(self, instance) -> typing.Optional[str]: # type: ignore[no-untyped-def] if getattr(instance, "feature_segment_id", None) is not None: return "SEGMENT" - elif getattr( - instance, "identity_id", None - ) or instance.feature.name in self.context.get("identity_feature_names", []): + elif getattr(instance, "identity_id", None) or isinstance( + instance, FeatureStateModel + ): + # An edge identity's overrides are the only states reaching this + # serialiser that are not ORM rows. return "IDENTITY" return None diff --git a/api/environments/identities/views.py b/api/environments/identities/views.py index 4e07c99b04d7..5af5c73f3d1b 100644 --- a/api/environments/identities/views.py +++ b/api/environments/identities/views.py @@ -34,6 +34,7 @@ IdentifyWithTraitsSerializer, IdentitySerializerWithTraitsAndSegments, ) +from evaluation.services import get_identity_feature_states from features.serializers import SDKIdentityFeatureStateSerializer from integrations.integration import identify_integrations from util.views import SDKAPIView @@ -134,7 +135,7 @@ def get(self, request, identifier, *args, **kwargs): # type: ignore[no-untyped- "IdentityTraitFlagsSegments", ("flags", "traits", "segments") ) identity_flags_traits_segments = IdentityFlagsWithTraitsAndSegments( - flags=identity.get_all_feature_states(), + flags=get_identity_feature_states(identity), traits=traits_data, segments=identity.get_segments(), ) @@ -289,7 +290,8 @@ def _get_single_feature_state_response( ) -> Response: context = self.get_serializer_context() # type: ignore[no-untyped-call] - for feature_state in identity.get_all_feature_states( + for feature_state in get_identity_feature_states( + identity, additional_filters=self._get_additional_filters(), ): if feature_state.feature.name == feature_name: @@ -317,7 +319,8 @@ def _get_all_feature_states_for_user_response( # type: ignore[no-untyped-def] :param identity: Identity model to return feature states for :return: Response containing lists of both serialized flags and traits """ - all_feature_states = identity.get_all_feature_states( + all_feature_states = get_identity_feature_states( + identity, additional_filters=self._get_additional_filters(), ) serializer_class = self.get_serializer_class() diff --git a/api/environments/sdk/serializers.py b/api/environments/sdk/serializers.py index 78750f2350df..b021fb43e98f 100644 --- a/api/environments/sdk/serializers.py +++ b/api/environments/sdk/serializers.py @@ -17,6 +17,7 @@ get_transient_identity_and_traits, ) from environments.sdk.types import SDKTraitData +from evaluation.services import get_identity_feature_states from features.serializers import ( FeatureStateSerializerFull, SDKIdentityFeatureStateSerializer, @@ -177,7 +178,8 @@ def save(self, **kwargs): # type: ignore[no-untyped-def] sdk_trait_data=sdk_trait_data, ) - all_feature_states = identity.get_all_feature_states( + all_feature_states = get_identity_feature_states( + identity, traits=traits, additional_filters=self.context.get("feature_states_additional_filters"), ) diff --git a/api/evaluation/__init__.py b/api/evaluation/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/api/evaluation/mappers.py b/api/evaluation/mappers.py new file mode 100644 index 000000000000..9f2feabbfc3f --- /dev/null +++ b/api/evaluation/mappers.py @@ -0,0 +1,444 @@ +"""Mapping Django ORM models to a flag-engine `EvaluationContext`. + +Core API's whole contribution to flag evaluation: resolve the rows that are +current for an environment, lay them out as the engine expects, and let it +decide which override wins and which variant an identity lands in. +""" + +from collections.abc import Iterable +from math import inf +from operator import attrgetter +from typing import TYPE_CHECKING, NamedTuple + +from django.db.models import Prefetch, Q +from flag_engine.context import types as engine_types +from flag_engine.segments.constants import IS_SET +from flag_engine.segments.types import ConditionOperator, RuleType +from pydantic import TypeAdapter + +from evaluation.types import ( + EvaluationContext, + FeatureContext, + IdentityContext, + SegmentContext, +) +from features.types import FeatureEngineMetadata +from segments.types import SegmentEngineMetadata + +if TYPE_CHECKING: + from edge_api.identities.models import EdgeIdentity + from environments.identities.models import Identity + from environments.identities.traits.models import Trait + from environments.models import Environment + from features.models import FeatureState + from features.multivariate.models import MultivariateFeatureStateValue + from segments.models import Condition, Segment, SegmentRule + from util.engine_models.features.models import ( + FeatureStateModel, + MultivariateFeatureStateValueModel, + ) + + +__all__ = ( + "IDENTITY_OVERRIDES_SEGMENT_KEY", + "IDENTITY_OVERRIDES_SEGMENT_NAME", + "map_condition_to_segment_condition", + "map_environment_to_evaluation_context", + "map_edge_identity_to_identity_context", + "map_engine_feature_state_to_feature_context", + "map_feature_state_to_feature_context", + "map_identity_overrides_to_segment_context", + "map_identity_to_identity_context", + "map_rule_to_segment_rule", + "map_segment_to_segment_context", +) + + +IDENTITY_OVERRIDES_SEGMENT_KEY = IDENTITY_OVERRIDES_SEGMENT_NAME = "identity_overrides" + + +_rule_type_adapter: TypeAdapter[RuleType] = TypeAdapter(RuleType) +_condition_operator_adapter: TypeAdapter[ConditionOperator] = TypeAdapter( + ConditionOperator +) + + +def map_environment_to_evaluation_context( + *, + environment: "Environment", + identity: "Identity | None" = None, + identity_context: "IdentityContext | None" = None, + traits: "Iterable[Trait] | None" = None, + segments: "Iterable[Segment] | None" = None, + additional_filters: "Q | None" = None, + from_replica: bool = False, +) -> EvaluationContext: + """Map Django ORM models to a flag-engine `EvaluationContext`. + + Resolves the feature states that are current for `environment` — defaults, + segment overrides, and `identity`'s own overrides — and lays them out as + `$.features` plus the overrides carried on each segment. + + Each feature context carries the row it was built from as metadata, which + the engine hands back on the corresponding `FlagResult`, so a caller still + working in Django rows never has to work out which override won. + + :param identity: the identity being evaluated, whose overrides are read + from the ORM along with the environment's own feature states. + :param identity_context: who is being evaluated, for an identity that is + not an ORM row and so has no overrides to read from it. An edge + identity keeps both its traits and its overrides in DynamoDB. + :param segments: segments to evaluate. + """ + context: EvaluationContext = { + "environment": { + "key": environment.api_key, + "name": environment.name or "", + }, + } + if identity_context is not None: + context["identity"] = identity_context + elif identity is not None: + context["identity"] = map_identity_to_identity_context( + identity, + environment=environment, + traits=traits, + ) + + ( + feature_states, + identity_overrides, + segment_overrides, + mv_fs_values_by_feature_state_id, + ) = _resolve_feature_states( + environment=environment, + identity=identity, + additional_filters=additional_filters, + from_replica=from_replica, + ) + + # No reading from ORM past this point! + + def to_feature_context( + feature_state: "FeatureState", + *, + priority: float | None = None, + ) -> FeatureContext: + return map_feature_state_to_feature_context( + feature_state, + mv_fs_values=mv_fs_values_by_feature_state_id.get(feature_state.pk), + priority=priority, + ) + + if segments is not None: + context["segments"] = { + str(segment.pk): map_segment_to_segment_context( + segment, + overrides=[ + to_feature_context(feature_state) + for feature_state in segment_overrides.get(segment.pk) or () + ], + ) + for segment in segments + } + + if identity_overrides: + # An identity override outranks every segment override, which the + # engine expresses as a priority no segment can beat. + context.setdefault("segments", {})[IDENTITY_OVERRIDES_SEGMENT_KEY] = ( + map_identity_overrides_to_segment_context( + [ + to_feature_context(feature_state, priority=-inf) + for feature_state in identity_overrides + ] + ) + ) + + context["features"] = { + (feature_context := to_feature_context(feature_state))["name"]: feature_context + for feature_state in feature_states + } + + return context + + +class _ResolvedFeatureStates(NamedTuple): + feature_states: list["FeatureState"] + identity_overrides: list["FeatureState"] + segment_overrides: dict[int, list["FeatureState"]] + mv_fs_values_by_feature_state_id: dict[ + int, "Iterable[MultivariateFeatureStateValue]" + ] + + +def _resolve_feature_states( + *, + environment: "Environment", + identity: "Identity | None", + additional_filters: "Q | None", + from_replica: bool, +) -> _ResolvedFeatureStates: + """Read the feature states current for `environment`, split by what they override.""" + # Deferred: `environments.models` imports this module's package. + from features.multivariate.models import MultivariateFeatureStateValue + from features.versioning.versioning_service import get_environment_flags_list + + override_filters = Q(identity__isnull=True) + if identity is not None and identity.pk: + # The identity is persisted (non-transient). + # Look for its identity overrides in addition to segment overrides. + override_filters = Q(identity=identity) | override_filters + if additional_filters: + override_filters &= additional_filters + + feature_states = get_environment_flags_list( + environment=environment, + additional_filters=override_filters, + from_replica=from_replica, + additional_select_related_args=["feature_segment__segment"], + additional_prefetch_related_args=[ + Prefetch( + "multivariate_feature_state_values", + queryset=MultivariateFeatureStateValue.objects.select_related( + "multivariate_feature_option" + ), + ) + ], + ) + + resolved = _ResolvedFeatureStates([], [], {}, {}) + + for feature_state in feature_states: + resolved.mv_fs_values_by_feature_state_id[feature_state.pk] = ( + feature_state.multivariate_feature_state_values.all() + ) + if feature_state.identity_id is not None: + resolved.identity_overrides.append(feature_state) + elif (feature_segment := feature_state.feature_segment) is not None: + resolved.segment_overrides.setdefault( + feature_segment.segment_id, [] + ).append(feature_state) + else: + resolved.feature_states.append(feature_state) + + return resolved + + +def map_identity_to_identity_context( + identity: "Identity", + *, + environment: "Environment", + traits: "Iterable[Trait] | None" = None, +) -> "IdentityContext": + """Map a Django ORM Identity to a flag-engine IdentityContext TypedDict.""" + trait_items: "Iterable[Trait]" = ( + traits + if traits is not None + # A transient identity was never persisted, so it has no stored traits + # to read, and asking for them would raise. + else identity.identity_traits.all() + if identity.pk + else () + ) + identity_traits = {trait.trait_key: trait.trait_value for trait in trait_items} + if identity.system_traits: + # System-owned traits are not user data: on a key clash, the system + # value wins. + identity_traits.update(identity.system_traits) + return { + "identifier": identity.identifier, + "key": identity.get_hash_key( + environment.use_identity_composite_key_for_hashing + ), + "traits": identity_traits, + } + + +def map_edge_identity_to_identity_context( + edge_identity: "EdgeIdentity", + *, + environment: "Environment", +) -> "IdentityContext": + """Map an edge identity, read back from DynamoDB, to an IdentityContext.""" + identity_model = edge_identity.engine_identity_model + return { + "identifier": edge_identity.identifier, + "key": edge_identity.get_hash_key( + environment.use_identity_composite_key_for_hashing + ), + "traits": { + trait.trait_key: trait.trait_value + for trait in identity_model.identity_traits + } + # System-owned traits are not user data: on a key clash, the system + # value wins. + | (identity_model.system_traits or {}), + } + + +def map_feature_state_to_feature_context( + feature_state: "FeatureState", + *, + mv_fs_values: "Iterable[MultivariateFeatureStateValue] | None" = None, + priority: float | None = None, +) -> FeatureContext: + """Map a Django ORM FeatureState to a flag-engine FeatureContext TypedDict.""" + feature = feature_state.feature + feature_context: FeatureContext = { + # The engine seeds multivariate variant allocation on the feature + # context key, so it has to be the bucketing seed rather than the + # feature state id, or recreating a feature state would move every + # enrolled identity to a different variant. See issue #7913. + "key": str(feature_state.mv_hashing_seed), + "name": feature.name, + "enabled": feature_state.enabled, + # Deliberately unparameterised by identity: picking a multivariate + # value is the engine's job now. + "value": feature_state.get_feature_state_value(), + "metadata": FeatureEngineMetadata(feature_state=feature_state), + } + + if variants := _map_mv_fs_values_to_feature_values(mv_fs_values or ()): + feature_context["variants"] = variants + + if priority is not None: + feature_context["priority"] = priority + elif (feature_segment := feature_state.feature_segment) is not None: + feature_context["priority"] = feature_segment.priority + + return feature_context + + +def _map_mv_fs_values_to_feature_values( + mv_fs_values: "Iterable[MultivariateFeatureStateValue]", +) -> list[engine_types.FeatureValue]: + # Ordered by id, and weighted by position in that order, because that is + # the order Core API has always allocated percentages in. The engine + # orders by `priority`, so the two only agree if we hand it the id order. + feature_values: list[engine_types.FeatureValue] = [] + for index, mv_fs_value in enumerate(sorted(mv_fs_values, key=attrgetter("id"))): + mv_option = mv_fs_value.multivariate_feature_option + feature_value: engine_types.FeatureValue = { + "value": mv_option.value, + "weight": mv_fs_value.percentage_allocation, + "priority": index, + } + if mv_option.key is not None: + # An unkeyed option resolves to a null variant, as it does today. + feature_value["key"] = mv_option.key + feature_values.append(feature_value) + return feature_values + + +def map_segment_to_segment_context( + segment: "Segment", + *, + overrides: "list[FeatureContext] | None" = None, +) -> SegmentContext: + """Map a Django ORM Segment to a flag-engine SegmentContext TypedDict.""" + segment_context: SegmentContext = { + "key": str(segment.pk), + "name": segment.name, + "rules": [map_rule_to_segment_rule(rule) for rule in segment.rules.all()], + "metadata": SegmentEngineMetadata(source="segment", pk=segment.pk), + } + if overrides: + segment_context["overrides"] = overrides + return segment_context + + +def map_engine_feature_state_to_feature_context( + feature_state: "FeatureStateModel", + *, + priority: float | None = None, +) -> FeatureContext: + """Map a DynamoDB-sourced FeatureStateModel to a FeatureContext TypedDict. + + An edge identity's overrides are stored rather than evaluated, so they + carry no bucketing salt: their own id seeds allocation, as it always has. + """ + feature_context: FeatureContext = { + "key": str(feature_state.django_id or feature_state.featurestate_uuid), + "name": feature_state.feature.name, + "enabled": feature_state.enabled, + "value": feature_state.feature_state_value, + "metadata": FeatureEngineMetadata(edge_feature_state=feature_state), + } + + if variants := _map_engine_mv_fs_values_to_feature_values( + feature_state.multivariate_feature_state_values + ): + feature_context["variants"] = variants + + if priority is not None: + feature_context["priority"] = priority + + return feature_context + + +def _map_engine_mv_fs_values_to_feature_values( + mv_fs_values: "Iterable[MultivariateFeatureStateValueModel]", +) -> list[engine_types.FeatureValue]: + # Ordered by id as the stored models always have been, falling back to the + # uuid for values that never reached the ORM. + feature_values: list[engine_types.FeatureValue] = [] + for index, mv_fs_value in enumerate( + sorted( + mv_fs_values, key=lambda mv_value: mv_value.id or mv_value.mv_fs_value_uuid + ) + ): + mv_option = mv_fs_value.multivariate_feature_option + feature_value: engine_types.FeatureValue = { + "value": mv_option.value, + "weight": mv_fs_value.percentage_allocation, + "priority": index, + } + if mv_option.key is not None: + feature_value["key"] = mv_option.key + feature_values.append(feature_value) + return feature_values + + +def map_identity_overrides_to_segment_context( + overrides: "list[FeatureContext]", +) -> SegmentContext: + """Express identity overrides as a segment matching the current identity.""" + return { + "key": IDENTITY_OVERRIDES_SEGMENT_KEY, + "name": IDENTITY_OVERRIDES_SEGMENT_NAME, + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.identity.key", + "operator": IS_SET, + "value": "", + } + ], + } + ], + "overrides": overrides, + "metadata": SegmentEngineMetadata(source="identity_overrides"), + } + + +def map_rule_to_segment_rule(rule: "SegmentRule") -> engine_types.SegmentRule: + return { + "type": _rule_type_adapter.validate_python(rule.type), + "conditions": [ + map_condition_to_segment_condition(condition) + for condition in rule.conditions.all() + ], + "rules": [map_rule_to_segment_rule(sub_rule) for sub_rule in rule.rules.all()], + } + + +def map_condition_to_segment_condition( + condition: "Condition", +) -> engine_types.StrValueSegmentCondition: + return { + "property": condition.property or "", + "operator": _condition_operator_adapter.validate_python(condition.operator), + "value": condition.value or "", + } diff --git a/api/evaluation/services.py b/api/evaluation/services.py new file mode 100644 index 000000000000..5fbf164e0993 --- /dev/null +++ b/api/evaluation/services.py @@ -0,0 +1,181 @@ +from math import inf +from typing import TYPE_CHECKING + +from django.db.models import Q +from flag_engine.engine import get_evaluation_result + +from evaluation.mappers import ( + IDENTITY_OVERRIDES_SEGMENT_KEY, + map_edge_identity_to_identity_context, + map_engine_feature_state_to_feature_context, + map_environment_to_evaluation_context, + map_identity_overrides_to_segment_context, +) +from evaluation.types import IdentityEvaluation + +if TYPE_CHECKING: + from edge_api.identities.models import EdgeIdentity + from environments.identities.models import Identity + from environments.identities.traits.models import Trait + from environments.models import Environment + from features.models import FeatureState + from segments.models import Segment + from util.engine_models.features.models import FeatureStateModel + + +__all__ = ( + "evaluate_identity", + "get_environment_feature_states", + "get_edge_identity_feature_states", + "get_edge_identity_segments", + "get_identity_feature_states", +) + + +def evaluate_identity( + identity: "Identity", + *, + traits: "list[Trait] | None" = None, + additional_filters: Q | None = None, +) -> IdentityEvaluation: + """Evaluate every flag in `identity`'s environment for that identity.""" + environment: "Environment" = identity.environment + context = map_environment_to_evaluation_context( + environment=environment, + identity=identity, + traits=traits, + segments=environment.get_segments_from_cache(), + additional_filters=additional_filters, + ) + result = get_evaluation_result(context) + + # Hand back the rows the engine ruled on, carrying its verdict, so that + # callers neither re-resolve a value nor work out which row won. + feature_states = [] + for flag in result["flags"].values(): + feature_state = flag["metadata"]["feature_state"] + feature_state.flag_result = flag + feature_states.append(feature_state) + + return IdentityEvaluation(result, feature_states) + + +def get_identity_feature_states( + identity: "Identity", + *, + traits: "list[Trait] | None" = None, + additional_filters: Q | None = None, +) -> "list[FeatureState]": + """The feature states to serve `identity`, one per feature. + + Each carries the engine's verdict on `flag_result`, so a caller reads the + evaluated value and variant off the row rather than resolving them again. + """ + _, feature_states = evaluate_identity( + identity, + traits=traits, + additional_filters=additional_filters, + ) + + if identity.environment.get_hide_disabled_flags() is True: + return [ + feature_state for feature_state in feature_states if feature_state.enabled + ] + + return feature_states + + +def get_edge_identity_feature_states( + edge_identity: "EdgeIdentity", +) -> "list[FeatureState | FeatureStateModel]": + """The feature states to serve an edge identity, one per feature. + + An edge identity's own overrides live in DynamoDB rather than the ORM, so + they are laid over the evaluated environment afterwards, and are the only + states in the returned list not carrying a `flag_result`. + """ + environment: "Environment" = edge_identity.environment + + context = map_environment_to_evaluation_context( + environment=environment, + identity_context=map_edge_identity_to_identity_context( + edge_identity, environment=environment + ), + segments=environment.get_segments_from_cache(), + ) + # The identity's own overrides are read back from DynamoDB rather than the + # ORM, so the mapper never saw them. They reach the engine the way every + # identity override does, as a segment no other override can outrank. + if overrides := [ + map_engine_feature_state_to_feature_context(feature_state, priority=-inf) + for feature_state in edge_identity.feature_overrides + ]: + context.setdefault("segments", {})[IDENTITY_OVERRIDES_SEGMENT_KEY] = ( + map_identity_overrides_to_segment_context(overrides) + ) + + feature_states: list["FeatureState | FeatureStateModel"] = [] + for flag in get_evaluation_result(context)["flags"].values(): + if (feature_state := flag["metadata"].get("feature_state")) is not None: + feature_state.flag_result = flag + feature_states.append(feature_state) + else: + # A stored model cannot be assigned the engine's verdict, so an + # edge identity's own overrides are still resolved by the + # serialiser. See Flagsmith/flagsmith-engine#340. + feature_states.append(flag["metadata"]["edge_feature_state"]) + + return feature_states + + +def get_edge_identity_segments(edge_identity: "EdgeIdentity") -> "list[Segment]": + """The segments an edge identity belongs to.""" + environment: "Environment" = edge_identity.environment + segments: "list[Segment]" = environment.project.get_segments_from_cache() + segments_by_pk = {segment.pk: segment for segment in segments} + + context = map_environment_to_evaluation_context( + environment=environment, + identity_context=map_edge_identity_to_identity_context( + edge_identity, environment=environment + ), + segments=segments, + ) + + return [ + segments_by_pk[pk] + for segment_result in get_evaluation_result(context)["segments"] + if (pk := segment_result["metadata"].get("pk")) is not None + ] + + +def get_environment_feature_states( + environment: "Environment", + *, + additional_filters: Q | None = None, + from_replica: bool = False, +) -> "list[FeatureState]": + """The feature states to serve for an environment, one per feature. + + Evaluated without an identity, so a segment whose rules read a trait or + split on the identity key cannot match. One reading `$.environment`, or + another flag, still can — as it does for an SDK evaluating locally. + """ + context = map_environment_to_evaluation_context( + environment=environment, + segments=environment.get_segments_from_cache(), + additional_filters=additional_filters, + from_replica=from_replica, + ) + result = get_evaluation_result(context) + + hide_disabled_flags = environment.get_hide_disabled_flags() is True + feature_states = [] + for flag in result["flags"].values(): + if hide_disabled_flags and not flag["enabled"]: + continue + feature_state = flag["metadata"]["feature_state"] + feature_state.flag_result = flag + feature_states.append(feature_state) + + return feature_states diff --git a/api/evaluation/types.py b/api/evaluation/types.py new file mode 100644 index 000000000000..49b4d587f07c --- /dev/null +++ b/api/evaluation/types.py @@ -0,0 +1,41 @@ +from typing import TYPE_CHECKING, NamedTuple, TypeAlias + +from flag_engine.context import types as context_types +from flag_engine.result import types as result_types + +from features.types import FeatureEngineMetadata +from segments.types import SegmentEngineMetadata + +if TYPE_CHECKING: + from features.models import FeatureState + + +__all__ = ( + "EvaluationContext", + "EvaluationResult", + "FeatureContext", + "FlagResult", + "IdentityContext", + "IdentityEvaluation", + "SegmentContext", +) + +EvaluationContext: TypeAlias = context_types.EvaluationContext[ + SegmentEngineMetadata, FeatureEngineMetadata +] +SegmentContext: TypeAlias = context_types.SegmentContext[ + SegmentEngineMetadata, FeatureEngineMetadata +] +FeatureContext: TypeAlias = context_types.FeatureContext[FeatureEngineMetadata] +IdentityContext: TypeAlias = context_types.IdentityContext +EvaluationResult: TypeAlias = result_types.EvaluationResult[ + SegmentEngineMetadata, FeatureEngineMetadata +] +FlagResult: TypeAlias = result_types.FlagResult[FeatureEngineMetadata] + + +class IdentityEvaluation(NamedTuple): + result: EvaluationResult + #: The evaluated feature states, each carrying its `flag_result`, for + #: callers still working in Django rows. + feature_states: "list[FeatureState]" diff --git a/api/features/models.py b/api/features/models.py index eaaf75074503..aa0b79619c57 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -25,7 +25,6 @@ LifecycleModelMixin, hook, ) -from flag_engine.utils.hashing import get_hashed_percentage_for_object_ids from ordered_model.models import OrderedModelBase # type: ignore[import-untyped] from simple_history.models import HistoricalRecords # type: ignore[import-untyped] @@ -50,6 +49,7 @@ SoftDeleteExportableModel, abstract_base_auditable_model_factory, ) +from evaluation.types import FlagResult from features.constants import ENVIRONMENT, FEATURE_SEGMENT, IDENTITY from features.custom_lifecycle import CustomLifecycleModelMixin from features.feature_states.models import AbstractBaseFeatureValueModel @@ -525,6 +525,13 @@ class FeatureState( # Multivariate bucketing seed, kept stable across recreation (#7913) — see mv_hashing_seed. mv_hashing_salt = models.IntegerField(null=True, blank=True, default=None) + #: How flag-engine evaluated this feature state, set by + #: `Identity.get_all_feature_states`. Carried on the instance so that the + #: value and variant travel with the row they were resolved from, rather + #: than being recomputed per serialiser. `None` where the state was read + #: outside an evaluation, in which case only its stored value is meaningful. + flag_result: FlagResult | None = None + class Meta: ordering = ["id"] @@ -726,30 +733,23 @@ def generate_feature_state_value_data(self, value): # type: ignore[no-untyped-d self.get_feature_state_key_name(fsv_type): value, } - def get_feature_state_value_by_hash_key( - self, - identity_hash_key: typing.Union[str, int] = None, # type: ignore[assignment] - ) -> typing.Any: - feature_state_value = ( - self.get_multivariate_feature_state_value(identity_hash_key) # type: ignore[arg-type] - if self.feature.type == MULTIVARIATE and identity_hash_key - else getattr(self, "feature_state_value", None) - ) + def get_feature_state_value(self) -> typing.Any: + """This state's stored value, before any evaluation. - # return the value of the feature state value only if the feature state - # has a related feature state value. Note that we use getattr rather than - # hasattr as we want to return None if no feature state value exists. + Multivariate allocation is flag-engine's job: read `flag_result` for + the value an identity actually sees. + """ + # Use getattr rather than hasattr, to return None where the feature + # state has no related feature state value. + feature_state_value = getattr(self, "feature_state_value", None) return feature_state_value and feature_state_value.value - def get_feature_state_value(self, identity: "Identity" = None) -> typing.Any: # type: ignore[assignment] - identity_hash_key = ( - identity.get_hash_key( - identity.environment.use_identity_composite_key_for_hashing - ) - if identity - else None - ) - return self.get_feature_state_value_by_hash_key(identity_hash_key) # type: ignore[arg-type] + @property + def evaluated_value(self) -> typing.Any: + """The value as evaluated for an identity, where one was evaluated.""" + if (flag_result := self.flag_result) is not None: + return flag_result["value"] + return self.get_feature_state_value() def get_feature_state_value_defaults(self) -> dict[str, typing.Any]: if ( @@ -805,36 +805,6 @@ def get_superseded_live_feature_state(self) -> typing.Optional["FeatureState"]: ) return superseded - def get_multivariate_feature_state_value( - self, identity_hash_key: str - ) -> AbstractBaseFeatureValueModel: - # the multivariate_feature_state_values should be prefetched at this point - # so we just convert them to a list and use python operations from here to - # avoid further queries to the DB - mv_options = list(self.multivariate_feature_state_values.all()) - - percentage_value = get_hashed_percentage_for_object_ids( - [self.mv_hashing_seed, identity_hash_key] - ) - - # Iterate over the mv options in order of id (so we get the same value each - # time) to determine the correct value to return to the identity based on - # the percentage allocations of the multivariate options. This gives us a - # way to ensure that the same value is returned every time we use the same - # percentage value. - start_percentage = 0 - for mv_option in sorted(mv_options, key=lambda o: o.id): - limit = getattr(mv_option, "percentage_allocation", 0) + start_percentage - if start_percentage <= percentage_value < limit: - return mv_option.multivariate_feature_option - - start_percentage = limit - - # if none of the percentage allocations match the percentage value we got for - # the identity, then we just return the default feature state value (or None - # if there isn't one - although this should never happen) - return getattr(self, "feature_state_value", None) # type: ignore[return-value] - @hook(BEFORE_CREATE) @hook(BEFORE_SAVE, when="deleted", is_not=True) def check_for_duplicate_feature_state(self): # type: ignore[no-untyped-def] diff --git a/api/features/serializers.py b/api/features/serializers.py index ebd4372e571e..f2ab64654397 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -50,7 +50,7 @@ DeleteBeforeUpdateWritableNestedModelSerializer, ) -from .constants import CONTROL_VARIANT_KEY, INTERSECTION, UNION +from .constants import INTERSECTION, UNION from .feature_lifecycle.types import LifecycleStage from .feature_segments.limits import ( SEGMENT_OVERRIDE_LIMIT_EXCEEDED_MESSAGE, @@ -61,7 +61,6 @@ ) from .feature_types import FEATURE_TYPE_CHOICES, MULTIVARIATE from .models import Feature, FeatureState -from .multivariate.models import MultivariateFeatureOption from .multivariate.serializers import NestedMultivariateFeatureOptionSerializer @@ -81,7 +80,7 @@ class Meta: @extend_schema_field({"type": ["string", "integer", "boolean"], "nullable": True}) def get_feature_state_value(self, obj): # type: ignore[no-untyped-def] - return obj.get_feature_state_value(identity=self.context.get("identity")) + return obj.evaluated_value class FeatureQuerySerializer(serializers.Serializer): # type: ignore[type-arg] @@ -589,7 +588,7 @@ class Meta: } ) def get_feature_state_value(self, obj): # type: ignore[no-untyped-def] - return obj.get_feature_state_value(identity=self.context.get("identity")) + return obj.evaluated_value class FeatureOwnerInputSerializer(UserIdsSerializer): @@ -656,17 +655,11 @@ class Meta(SDKFeatureStateSerializer.Meta): @extend_schema_field({"type": "string", "nullable": True}) def get_variant(self, obj: FeatureState) -> str | None: - if obj.feature.type != MULTIVARIATE: + if obj.feature.type != MULTIVARIATE or obj.flag_result is None: return None - identity = self.context["identity"] - value_object = obj.get_multivariate_feature_state_value( - identity.get_hash_key( - identity.environment.use_identity_composite_key_for_hashing - ) - ) - if isinstance(value_object, MultivariateFeatureOption): - return value_object.key - return CONTROL_VARIANT_KEY + # The engine reports the control bucket as `CONTROL_VARIANT_KEY`, and + # an unkeyed variant as None, which is what this returned before. + return obj.flag_result["variant"] @cached_property def _build_metadata(self) -> Callable[[FeatureState], dict[str, Any] | None]: @@ -710,7 +703,7 @@ class Meta: } ) def get_feature_state_value(self, obj): # type: ignore[no-untyped-def] - return obj.get_feature_state_value(identity=self.context.get("identity")) + return obj.evaluated_value def save(self, **kwargs): # type: ignore[no-untyped-def] try: diff --git a/api/features/types.py b/api/features/types.py new file mode 100644 index 000000000000..d82a1b506eb3 --- /dev/null +++ b/api/features/types.py @@ -0,0 +1,26 @@ +from typing import TYPE_CHECKING + +from typing_extensions import NotRequired, TypedDict + +if TYPE_CHECKING: + from features.models import FeatureState + from util.engine_models.features.models import FeatureStateModel + + +class FeatureEngineMetadata(TypedDict): + """Core API data carried on a `FeatureContext` and back on a `FlagResult`. + + The engine treats this as opaque, so the feature state an evaluated flag + came from can simply ride along, saving callers from working out which + override won. Exactly one of the two is set, naming where it was read + from. + + The annotations are deliberately forward references: nothing here may + import Django at runtime, or `features.models` could not annotate against + it. + """ + + feature_state: NotRequired["FeatureState"] + #: An edge identity's own overrides are stored in DynamoDB rather than the + #: ORM, so they reach evaluation as the model they were read back as. + edge_feature_state: NotRequired["FeatureStateModel"] diff --git a/api/features/views.py b/api/features/views.py index db62e6f445f1..2999f4ff66d8 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -63,6 +63,10 @@ EnvironmentKeyPermissions, NestedEnvironmentPermissions, ) +from evaluation.services import ( + get_environment_feature_states, + get_identity_feature_states, +) from features.dependencies.services import validate_segment_flag_dependencies from features.feature_lifecycle.services import ( annotate_feature_queryset_with_lifecycle_stage, @@ -882,7 +886,7 @@ def get_queryset(self): # type: ignore[no-untyped-def] @action(methods=["GET"], detail=False) def all(self, request, *args, **kwargs): # type: ignore[no-untyped-def] identity = get_object_or_404(Identity, pk=self.kwargs["identity_pk"]) - feature_states = identity.get_all_feature_states() + feature_states = get_identity_feature_states(identity) serializer = IdentityAllFeatureStatesSerializer( instance=feature_states, @@ -1047,27 +1051,34 @@ def get(self, request, identifier=None, *args, **kwargs): # type: ignore[no-unt if identifier: return self._get_flags_response_with_identifier(request, identifier) - if "feature" in request.GET: - feature_states = get_environment_flags_list( - environment=request.environment, - feature_name=request.GET["feature"], - additional_filters=self._additional_filters, - from_replica=True, + if feature_name := request.GET.get("feature"): + # Filtered after evaluating, not before, because of dependent flags + feature_state = next( + ( + feature_state + for feature_state in get_environment_feature_states( + request.environment, + additional_filters=self._additional_filters, + from_replica=True, + ) + if feature_state.feature.name == feature_name + ), + None, ) - if not feature_states: + if feature_state is None: return Response( {"detail": "Given feature not found"}, status=status.HTTP_404_NOT_FOUND, ) - return Response(self.get_serializer(feature_states[0]).data) + return Response(self.get_serializer(feature_state).data) if settings.CACHE_FLAGS_SECONDS > 0: - data = self._get_flags_from_cache(request.environment, from_replica=True) + data = self._get_flags_from_cache(request.environment) else: data = self.get_serializer( - get_environment_flags_list( - environment=request.environment, + get_environment_feature_states( + request.environment, additional_filters=self._additional_filters, from_replica=True, ), @@ -1081,21 +1092,17 @@ def get(self, request, identifier=None, *args, **kwargs): # type: ignore[no-unt ) @property - def _additional_filters(self) -> Q: - filters = Q(feature_segment=None, identity=None) - - if self.request.environment.get_hide_disabled_flags() is True: - return filters & Q(enabled=True) - + def _additional_filters(self) -> Q | None: + # Disabled flags are hidden after evaluating, so that a disabled + # override is not passed over in favour of an enabled default. if self.request.originated_from is RequestOrigin.CLIENT: - return filters & Q(feature__is_server_key_only=False) + return Q(feature__is_server_key_only=False) - return filters + return None def _get_flags_from_cache( self, environment: Environment, - from_replica: bool = False, ) -> list[typing.Any]: data: list[typing.Any] # Include request origin in cache key to isolate client vs server requests @@ -1103,10 +1110,10 @@ def _get_flags_from_cache( data = flags_cache.get(cache_key) if not data: data = self.get_serializer( - get_environment_flags_list( - environment=environment, + get_environment_feature_states( + environment, additional_filters=self._additional_filters, - from_replica=from_replica, + from_replica=True, ), many=True, ).data @@ -1130,19 +1137,29 @@ def _get_flags_response_with_identifier( ) replace_identity_environment(identity, request.environment) + feature_states = get_identity_feature_states(identity) + if feature_name := request.GET.get("feature"): - feature_states = identity.get_all_feature_states(feature_name=feature_name) - if not feature_states: + # Filtered after evaluation rather than before: a segment may + # depend on a flag other than the one asked for. + feature_state = next( + ( + feature_state + for feature_state in feature_states + if feature_state.feature.name == feature_name + ), + None, + ) + if feature_state is None: return Response( {"detail": "Given feature not found"}, status=status.HTTP_404_NOT_FOUND, ) return Response( - self.get_serializer(feature_states[0]).data, + self.get_serializer(feature_state).data, status=status.HTTP_200_OK, ) - feature_states = identity.get_all_feature_states() flags = self.get_serializer(feature_states, many=True) return Response(flags.data, status=status.HTTP_200_OK) diff --git a/api/integrations/amplitude/amplitude.py b/api/integrations/amplitude/amplitude.py index 7bf758e8c45e..e2f164726f63 100644 --- a/api/integrations/amplitude/amplitude.py +++ b/api/integrations/amplitude/amplitude.py @@ -38,7 +38,7 @@ def generate_user_data( feature_properties = {} for feature_state in feature_states: - value = feature_state.get_feature_state_value(identity=identity) + value = feature_state.evaluated_value feature_properties[feature_state.feature.name] = ( value if (feature_state.enabled and value is not None) diff --git a/api/integrations/heap/heap.py b/api/integrations/heap/heap.py index 5b15d4ca1c17..1e72e10157fb 100644 --- a/api/integrations/heap/heap.py +++ b/api/integrations/heap/heap.py @@ -33,7 +33,7 @@ def generate_user_data( feature_properties = {} for feature_state in feature_states: - value = feature_state.get_feature_state_value(identity=identity) + value = feature_state.evaluated_value feature_properties[feature_state.feature.name] = ( value if (feature_state.enabled and value is not None) diff --git a/api/integrations/mixpanel/mixpanel.py b/api/integrations/mixpanel/mixpanel.py index cc2820aaa53c..eea1853348c8 100644 --- a/api/integrations/mixpanel/mixpanel.py +++ b/api/integrations/mixpanel/mixpanel.py @@ -45,7 +45,7 @@ def generate_user_data( feature_properties = {} for feature_state in feature_states: - value = feature_state.get_feature_state_value(identity=identity) + value = feature_state.evaluated_value feature_properties[feature_state.feature.name] = ( value if (feature_state.enabled and value) else feature_state.enabled ) diff --git a/api/integrations/rudderstack/rudderstack.py b/api/integrations/rudderstack/rudderstack.py index 1c6a248ca8c7..e5b93c978161 100644 --- a/api/integrations/rudderstack/rudderstack.py +++ b/api/integrations/rudderstack/rudderstack.py @@ -30,7 +30,7 @@ def generate_user_data( feature_properties = {} for feature_state in feature_states: - value = feature_state.get_feature_state_value(identity=identity) + value = feature_state.evaluated_value feature_properties[feature_state.feature.name] = ( value if (feature_state.enabled and value) else feature_state.enabled ) diff --git a/api/integrations/segment/segment.py b/api/integrations/segment/segment.py index a2ba93ce1863..e1d03f2da5f7 100644 --- a/api/integrations/segment/segment.py +++ b/api/integrations/segment/segment.py @@ -31,7 +31,7 @@ def generate_user_data( feature_properties = {} for feature_state in feature_states: - value = feature_state.get_feature_state_value(identity=identity) + value = feature_state.evaluated_value feature_properties[feature_state.feature.name] = ( value if (feature_state.enabled and value is not None) diff --git a/api/integrations/webhook/serializers.py b/api/integrations/webhook/serializers.py index b844ff6b7308..7238b31a9614 100644 --- a/api/integrations/webhook/serializers.py +++ b/api/integrations/webhook/serializers.py @@ -4,12 +4,12 @@ from flag_engine.engine import get_evaluation_result from rest_framework import serializers +from evaluation.mappers import map_environment_to_evaluation_context from features.serializers import FeatureStateSerializerFull from integrations.common.serializers import ( BaseEnvironmentIntegrationModelSerializer, ) from segments.models import Segment -from util.mappers.engine import map_environment_to_evaluation_context from .models import WebhookConfiguration diff --git a/api/segments/types.py b/api/segments/types.py index 57bb09634974..ed6ad817c403 100644 --- a/api/segments/types.py +++ b/api/segments/types.py @@ -1,9 +1,18 @@ +from typing import Literal + from flag_engine.segments.types import ConditionOperator, RuleType from typing_extensions import NotRequired, TypedDict +#: Where a `SegmentContext` came from. Identity overrides have no segment of +#: their own; they are expressed to the engine as a synthetic segment, so that +#: the engine resolves them by priority like any other override. +SegmentSource = Literal["segment", "identity_overrides"] + class SegmentEngineMetadata(TypedDict): - pk: int + source: SegmentSource + #: Absent on synthetic identity-override segments. + pk: NotRequired[int] class SegmentCondition(TypedDict): diff --git a/api/segments/views.py b/api/segments/views.py index 79a005b68351..a8a9dcfd64b4 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -3,6 +3,7 @@ import structlog from common.environments.permissions import VIEW_IDENTITIES from common.projects.permissions import VIEW_PROJECT +from django.core.exceptions import ObjectDoesNotExist from django.db import models from django.db.models import Prefetch from django.utils.decorators import method_decorator @@ -21,6 +22,7 @@ from edge_api.identities.models import EdgeIdentity from environments.identities.models import Identity from environments.models import Environment +from evaluation.services import get_edge_identity_segments from features.models import FeatureState from features.serializers import ( AssociatedFeaturesQuerySerializer, @@ -133,12 +135,23 @@ def get_queryset(self): # type: ignore[no-untyped-def] identity_pk = query_serializer.validated_data.get("identity") if identity_pk: + segments: list[Segment] = [] if identity_pk.isdigit(): - identity = Identity.objects.get(pk=identity_pk) - segment_ids = [segment.id for segment in identity.get_segments()] + segments = Identity.objects.get(pk=identity_pk).get_segments() else: - segment_ids = EdgeIdentity.dynamo_wrapper.get_segment_ids(identity_pk) - queryset = queryset.filter(id__in=segment_ids) + try: + identity_document = EdgeIdentity.dynamo_wrapper.get_item_from_uuid( + identity_pk + ) + except ObjectDoesNotExist: + # An identity the environment has never seen belongs to no + # segments, rather than being an error. + pass + else: + segments = get_edge_identity_segments( + EdgeIdentity.from_identity_document(identity_document) + ) + queryset = queryset.filter(id__in=[segment.id for segment in segments]) search_term = query_serializer.validated_data.get("q") if search_term: diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 1fec4a7f847b..615116b19d35 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -77,6 +77,7 @@ UserEnvironmentPermission, UserPermissionGroupEnvironmentPermission, ) +from evaluation.services import evaluate_identity from experimentation.models import Experiment, ExperimentStatus from features.feature_external_resources.models import FeatureExternalResource from features.feature_types import MULTIVARIATE @@ -123,6 +124,7 @@ AdminClientAuthType, EnableFeaturesFixture, MigratorFactory, + VariantAssignmentFixture, WithEnvironmentPermissionsCallable, WithOrganisationPermissionsCallable, WithProjectPermissionsCallable, @@ -550,6 +552,29 @@ def environment_v2_versioning(environment: Environment) -> Environment: return environment +@pytest.fixture() +def variant_assignment() -> VariantAssignmentFixture: + """Which variant each identity is currently bucketed into. + + Goes through the same entry point the SDK endpoints do, so that a test + asserting an identity keeps its variant is asserting it of production + behaviour rather than of a probe written alongside it. + """ + + def _variant_assignment( + identities: list[Identity], + feature_name: str, + ) -> dict[str, str | None]: + return { + identity.identifier: evaluate_identity(identity).result["flags"][ + feature_name + ]["variant"] + for identity in identities + } + + return _variant_assignment + + @pytest.fixture() def identity(environment: Environment) -> Identity: return Identity.objects.create(identifier="test_identity", environment=environment) diff --git a/api/tests/integration/conftest.py b/api/tests/integration/conftest.py index c981b819e814..3b418641219f 100644 --- a/api/tests/integration/conftest.py +++ b/api/tests/integration/conftest.py @@ -20,7 +20,10 @@ from features.future.types import SegmentOverrideRequest, UpdateFlagRequest from organisations.models import Organisation from tests.integration.helpers import create_mv_option_with_api -from tests.types import CreateSegmentOverrideFixture +from tests.types import ( + CreateSegmentOverrideFixture, + SetMultivariateAllocationsFixture, +) from users.models import FFAdminUser @@ -29,6 +32,34 @@ def mv_option_value(): # type: ignore[no-untyped-def] return "test_mv_value" +@pytest.fixture() +def set_mv_allocations(admin_client: APIClient) -> SetMultivariateAllocationsFixture: + """Reallocate a multivariate feature state's variants, through the API.""" + + def _set_mv_allocations( + environment_api_key: str, + feature_state_id: int, + allocation_by_mv_option_id: dict[int, float], + ) -> None: + url = reverse( + "api-v1:environments:environment-featurestates-detail", + args=[environment_api_key, feature_state_id], + ) + feature_state_data = admin_client.get(url).json() + for mv_value in feature_state_data["multivariate_feature_state_values"]: + mv_value["percentage_allocation"] = allocation_by_mv_option_id[ + mv_value["multivariate_feature_option"] + ] + response = admin_client.put( + url, + data=json.dumps(feature_state_data), + content_type="application/json", + ) + assert response.status_code == status.HTTP_200_OK + + return _set_mv_allocations + + @pytest.fixture() def django_client(): # type: ignore[no-untyped-def] return DjangoClient() diff --git a/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py b/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py index 32e24c58352f..08c4d8044d5a 100644 --- a/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py +++ b/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py @@ -955,6 +955,8 @@ def test_put_identity_override__identity_not_found__creates_identity_with_overri assert feature_state_data["feature_state_value"] == expected_feature_state_value +@pytest.mark.parametrize("segment_condition_property", ["trait_key_1"]) +@pytest.mark.parametrize("segment_condition_value", ["trait_value_1"]) @pytest.mark.parametrize( "segment_override_type, segment_override_value", ( @@ -979,21 +981,9 @@ def test_get_all_feature_states__with_overrides__returns_correct_override_source segment_override_value, ): # Given - # Mock the get_segment_ids method so that it returns no segments for the first - # request (to get the environment default), then so that it returns one segment - # for the segment and identity override requests. - segment_ids_responses = [[], [segment], [segment]] - - def get_segment_ids_side_effect(*args, **kwargs): # type: ignore[no-untyped-def] - nonlocal segment_ids_responses - return segment_ids_responses.pop(0) - edge_identity_dynamo_wrapper_mock.get_item_from_uuid_or_404.return_value = ( identity_document_without_fs ) - edge_identity_dynamo_wrapper_mock.get_segment_ids.side_effect = ( - get_segment_ids_side_effect - ) get_all_identity_feature_states_url = reverse( "api-v1:environments:edge-identity-featurestates-all", diff --git a/api/tests/integration/environments/identities/test_integration_identities.py b/api/tests/integration/environments/identities/test_integration_identities.py index eaa98c8942ab..5ce938e1bfaf 100644 --- a/api/tests/integration/environments/identities/test_integration_identities.py +++ b/api/tests/integration/environments/identities/test_integration_identities.py @@ -26,21 +26,26 @@ ) -# mock the returned percentage for the identity to simulate them falling into each of -# the percentage allocation brackets for the feature variants +# Which variant an identity hashes into is not something an API test can pin +# down, so each case allocates a single variant the whole range instead. @pytest.mark.parametrize( - "hashed_percentage, expected_mv_value", + [ + "variant_1_allocation", + "variant_2_allocation", + "expected_value", + "expected_variant", + ], ( - (variant_1_percentage_allocation - 1, variant_1_value), - (total_variance_percentage - 1, variant_2_value), - (total_variance_percentage + 1, control_value), + pytest.param(100, 0, variant_1_value, "variant-1", id="all_variant_1"), + pytest.param(0, 100, variant_2_value, "variant-2", id="all_variant_2"), + pytest.param(0, 0, control_value, "control", id="unallocated_falls_through"), ), ) -@mock.patch("features.models.get_hashed_percentage_for_object_ids") -def test_get_feature_states_for_identity__mv_percentage_allocation__returns_correct_value( # type: ignore[no-untyped-def] - mock_get_hashed_percentage_value, - hashed_percentage, - expected_mv_value, +def test_get_feature_states_for_identity__mv_allocation__returns_value_and_variant( # type: ignore[no-untyped-def] + variant_1_allocation, + variant_2_allocation, + expected_value, + expected_variant, sdk_client, admin_client, project, @@ -48,11 +53,10 @@ def test_get_feature_states_for_identity__mv_percentage_allocation__returns_corr environment, identity, identity_identifier, + set_mv_allocations, ): - # Given / When - # Then - # Firstly, let's create some features to use - # one standard feature + # Given + # a standard (non-multivariate) feature standard_feature_initial_value = "control" standard_feature_id = create_feature_with_api( client=admin_client, @@ -61,7 +65,7 @@ def test_get_feature_states_for_identity__mv_percentage_allocation__returns_corr initial_value=standard_feature_initial_value, ) - # and one multivariate feature + # and a multivariate feature with two keyed variants multivariate_feature_id = create_feature_with_api( client=admin_client, project_id=project, @@ -69,13 +73,13 @@ def test_get_feature_states_for_identity__mv_percentage_allocation__returns_corr initial_value=control_value, feature_type=MULTIVARIATE, ) - # With two mv options - create_mv_option_with_api( + variant_1_mvfo_id = create_mv_option_with_api( admin_client, project, multivariate_feature_id, variant_1_percentage_allocation, variant_1_value, + key="variant-1", ) variant_2_mvfo_id = create_mv_option_with_api( admin_client, @@ -83,146 +87,48 @@ def test_get_feature_states_for_identity__mv_percentage_allocation__returns_corr multivariate_feature_id, variant_2_percentage_allocation, variant_2_value, + key="variant-2", ) - # Now, when we mock the hashed percentage that the user gets - # to avoid the randomness factor - mock_get_hashed_percentage_value.return_value = hashed_percentage - - # and request the flags for the identity base_identity_flags_url = reverse("api-v1:sdk-identities") identity_flags_url = f"{base_identity_flags_url}?identifier={identity_identifier}" - identity_response_1 = sdk_client.get(identity_flags_url) - - # Then, we get a result for both of the features we created - assert identity_response_1.status_code == status.HTTP_200_OK - identity_response_json = identity_response_1.json() - assert len(identity_response_json["flags"]) == 2 - - # and the correct values are returned for the features - values_dict = { - flag["feature"]["id"]: flag["feature_state_value"] - for flag in identity_response_json["flags"] - } - assert values_dict[standard_feature_id] == standard_feature_initial_value - assert values_dict[multivariate_feature_id] == expected_mv_value - - # Now, let's change the percentage allocations on the mv options - # first, we need to get the feature state id for the feature in the given - # environment - feature_state_id = next( - filter( - lambda flag: flag["feature"]["id"] == multivariate_feature_id, - identity_response_json["flags"], - ) - )["id"] - - # now let's get the existing data for the feature state so we can alter it and - # then PUT it back - feature_state_detail_url = reverse( - "api-v1:environments:environment-featurestates-detail", - args=[environment_api_key, feature_state_id], + flags = sdk_client.get(identity_flags_url).json()["flags"] + multivariate_feature_state_id = next( + flag["id"] for flag in flags if flag["feature"]["id"] == multivariate_feature_id ) - retrieve_feature_state_response = admin_client.get(feature_state_detail_url) - feature_state_data = retrieve_feature_state_response.json() - - # now let's amend the data so that all identities should receive variant 2 - for mv_value in feature_state_data["multivariate_feature_state_values"]: - if mv_value["multivariate_feature_option"] == variant_2_mvfo_id: - mv_value["percentage_allocation"] = 100 - else: - mv_value["percentage_allocation"] = 0 - - # and PUT the data back - update_feature_state_response = admin_client.put( - feature_state_detail_url, - data=json.dumps(feature_state_data), - content_type="application/json", - ) - assert update_feature_state_response.status_code == status.HTTP_200_OK - # Then when we get the flags for an identity, the multivariate feature returns the - # value of the 2nd variate - identity_response_2 = sdk_client.get(identity_flags_url) - values_dict = { - flag["feature"]["id"]: flag["feature_state_value"] - for flag in identity_response_2.json()["flags"] - } - assert values_dict[multivariate_feature_id] == variant_2_value + # When + # the whole range is allocated to one variant, or to neither + set_mv_allocations( + environment_api_key, + multivariate_feature_state_id, + { + variant_1_mvfo_id: variant_1_allocation, + variant_2_mvfo_id: variant_2_allocation, + }, + ) + response = sdk_client.get(identity_flags_url) + # Then + assert response.status_code == status.HTTP_200_OK + flags = response.json()["flags"] + assert len(flags) == 2 + flags_by_feature_id = {flag["feature"]["id"]: flag for flag in flags} -@pytest.mark.parametrize( - "hashed_percentage, expected_variant", - ( - (variant_1_percentage_allocation - 1, "variant-1"), - (total_variance_percentage - 1, "variant-2"), - (total_variance_percentage + 1, "control"), - ), -) -@mock.patch("features.models.get_hashed_percentage_for_object_ids") -def test_get_feature_states_for_identity__mv_allocation__returns_variant( # type: ignore[no-untyped-def] - mock_get_hashed_percentage_value, - hashed_percentage, - expected_variant, - sdk_client, - admin_client, - project, - environment_api_key, - environment, - identity, - identity_identifier, -): - # Given - # a standard (non-multivariate) feature - standard_feature_id = create_feature_with_api( - client=admin_client, - project_id=project, - feature_name="standard_feature", - initial_value="control", + # the multivariate flag serves the allocated variant, reporting its key, + # falling through to the control value when nothing is allocated + assert ( + flags_by_feature_id[multivariate_feature_id]["feature_state_value"] + == expected_value ) + assert flags_by_feature_id[multivariate_feature_id]["variant"] == expected_variant - # and a multivariate feature with two keyed variants spanning part of the range, - # so the remainder falls through to the control - multivariate_feature_id = create_feature_with_api( - client=admin_client, - project_id=project, - feature_name="multivariate_feature", - initial_value=control_value, - feature_type=MULTIVARIATE, - ) - create_mv_option_with_api( - admin_client, - project, - multivariate_feature_id, - variant_1_percentage_allocation, - variant_1_value, - key="variant-1", - ) - create_mv_option_with_api( - admin_client, - project, - multivariate_feature_id, - variant_2_percentage_allocation, - variant_2_value, - key="variant-2", + # and the standard flag is unaffected, and has no variant + assert ( + flags_by_feature_id[standard_feature_id]["feature_state_value"] + == standard_feature_initial_value ) - - # When - # the identity hashes into a known allocation band - mock_get_hashed_percentage_value.return_value = hashed_percentage - base_url = reverse("api-v1:sdk-identities") - url = f"{base_url}?identifier={identity_identifier}" - response = sdk_client.get(url) - - # Then - assert response.status_code == status.HTTP_200_OK - variant_by_feature = { - flag["feature"]["id"]: flag["variant"] for flag in response.json()["flags"] - } - # the multivariate flag reports the variant key (or "control" on fall-through) - assert variant_by_feature[multivariate_feature_id] == expected_variant - # and the standard flag has no variant - assert variant_by_feature[standard_feature_id] is None + assert flags_by_feature_id[standard_feature_id]["variant"] is None def test_get_flags__multivariate_feature__response_excludes_variant( # type: ignore[no-untyped-def] diff --git a/api/tests/types.py b/api/tests/types.py index ffbfd955bda5..7cf0104e17a7 100644 --- a/api/tests/types.py +++ b/api/tests/types.py @@ -2,6 +2,7 @@ from django_test_migrations.migrator import Migrator +from environments.identities.models import Identity from environments.permissions.models import UserEnvironmentPermission from organisations.permissions.models import UserOrganisationPermission from projects.models import UserProjectPermission @@ -46,6 +47,23 @@ class MigratorFactory(Protocol): def __call__(self, name: Optional[str] = None) -> Migrator: ... +class VariantAssignmentFixture(Protocol): + def __call__( + self, + identities: "list[Identity]", + feature_name: str, + ) -> dict[str, str | None]: ... + + +class SetMultivariateAllocationsFixture(Protocol): + def __call__( + self, + environment_api_key: str, + feature_state_id: int, + allocation_by_mv_option_id: dict[int, float], + ) -> None: ... + + class CreateSegmentOverrideFixture(Protocol): def __call__( self, diff --git a/api/tests/unit/edge_api/identities/test_edge_api_identities_serializers.py b/api/tests/unit/edge_api/identities/test_edge_api_identities_serializers.py index 49374ceef516..3ea6780c89c6 100644 --- a/api/tests/unit/edge_api/identities/test_edge_api_identities_serializers.py +++ b/api/tests/unit/edge_api/identities/test_edge_api_identities_serializers.py @@ -11,8 +11,9 @@ from environments.identities.serializers import ( IdentityAllFeatureStatesSerializer, ) +from evaluation.services import get_edge_identity_feature_states from features.feature_types import STANDARD -from features.models import Feature, FeatureState +from features.models import Feature from users.models import FFAdminUser from util.engine_models.features.models import FeatureModel, FeatureStateModel from util.mappers import map_identity_to_identity_document @@ -184,19 +185,31 @@ def test_edge_identity_feature_state_serializer__update_override__calls_webhook( def test_all_feature_states_serializer__edge_identity_with_mv_feature__uses_mv_values( # type: ignore[no-untyped-def] - identity, multivariate_feature, environment + identity, multivariate_feature, environment, mocker ): # Given identity_document = map_identity_to_identity_document(identity) del identity_document["django_id"] # delete django id to simulate an edge identity - identity_model = EdgeIdentity.from_identity_document(identity_document) + edge_identity = EdgeIdentity.from_identity_document(identity_document) - feature_state = FeatureState.objects.get( - feature=multivariate_feature, environment=environment - ) + mocker.patch( + "edge_api.identities.models.EdgeIdentity.dynamo_wrapper" + ).get_segment_ids.return_value = [] + + # The feature state has to come from an evaluation, which is what resolves + # the identity's multivariate value. + feature_states = get_edge_identity_feature_states(edge_identity) + (feature_state,) = [ + feature_state + for feature_state in feature_states + if feature_state.feature.name == multivariate_feature.name + ] serializer = IdentityAllFeatureStatesSerializer( - context={"identity": identity_model, "environment_api_key": environment.api_key} + context={ + "identity": edge_identity, + "environment_api_key": environment.api_key, + } ) # When diff --git a/api/tests/unit/edge_api/identities/test_edge_identity_models.py b/api/tests/unit/edge_api/identities/test_edge_identity_models.py index 46f1dfa0bdec..8cab5f364215 100644 --- a/api/tests/unit/edge_api/identities/test_edge_identity_models.py +++ b/api/tests/unit/edge_api/identities/test_edge_identity_models.py @@ -4,6 +4,7 @@ import pytest import shortuuid from django.utils import timezone +from flag_engine.segments.constants import EQUAL from freezegun import freeze_time from pytest_django import DjangoAssertNumQueries from pytest_django.fixtures import SettingsWrapper @@ -14,29 +15,52 @@ from api_keys.user import APIKeyUser from edge_api.identities.models import EdgeIdentity from environments.models import Environment +from evaluation.services import get_edge_identity_feature_states from features.models import Feature, FeatureSegment, FeatureState from features.versioning.tasks import enable_v2_versioning from features.workflows.core.models import ChangeRequest from projects.models import Project -from segments.models import Segment +from segments.models import Condition, Segment, SegmentRule from tests.types import EnableFeaturesFixture from users.models import FFAdminUser from util.engine_models.features.models import FeatureModel, FeatureStateModel +from util.engine_models.identities.models import IdentityModel +from util.engine_models.identities.traits.models import TraitModel + +MATCHING_TRAIT_KEY = "segment-membership" +MATCHING_TRAIT_VALUE = "yes" + + +def _create_matching_segment(project: Project, name: str) -> Segment: + """A segment every identity carrying the matching trait belongs to.""" + segment: Segment = Segment.objects.create(name=name, project=project) + Condition.objects.create( + rule=SegmentRule.objects.create(segment=segment, type=SegmentRule.ALL_RULE), + property=MATCHING_TRAIT_KEY, + operator=EQUAL, + value=MATCHING_TRAIT_VALUE, + ) + return segment + + +def _matching_identity_model(environment_api_key: str) -> IdentityModel: + return IdentityModel( + identifier="identity", + environment_api_key=environment_api_key, + identity_traits=[ + TraitModel(trait_key=MATCHING_TRAIT_KEY, trait_value=MATCHING_TRAIT_VALUE) + ], + ) def test_get_all_feature_states__multiple_segment_overrides__uses_segment_priorities( # type: ignore[no-untyped-def] - environment, project, segment, feature, mocker + environment, project, feature, mocker ): # Given - another_segment = Segment.objects.create(name="another_segment", project=project) - - edge_identity_dynamo_wrapper_mock = mocker.patch( - "edge_api.identities.models.EdgeIdentity.dynamo_wrapper", + # two segments the identity matches + segment, another_segment = ( + _create_matching_segment(project, name) for name in ("segment", "another") ) - edge_identity_dynamo_wrapper_mock.get_segment_ids.return_value = [ - segment.id, - another_segment.id, - ] feature_segment_p1 = FeatureSegment.objects.create( segment=segment, feature=feature, environment=environment, priority=1 @@ -52,22 +76,15 @@ def test_get_all_feature_states__multiple_segment_overrides__uses_segment_priori feature=feature, environment=environment, feature_segment=feature_segment_p2 ) - identity_model = mocker.MagicMock( - environment_api_key=environment.api_key, identity_features=[] - ) - edge_identity = EdgeIdentity(identity_model) + edge_identity = EdgeIdentity(_matching_identity_model(environment.api_key)) # When - feature_states, _ = edge_identity.get_all_feature_states() + feature_states = get_edge_identity_feature_states(edge_identity) # Then assert len(feature_states) == 1 assert feature_states[0] == segment_override_p1 - edge_identity_dynamo_wrapper_mock.get_segment_ids.assert_called_once_with( - identity_model=identity_model - ) - def test_get_all_feature_states__not_live_change_request__ignores_not_live_states( # type: ignore[no-untyped-def] environment, project, segment, feature, feature_state, admin_user, mocker @@ -96,7 +113,7 @@ def test_get_all_feature_states__not_live_change_request__ignores_not_live_state # When with freeze_time(timezone.now() + timedelta(hours=2)): - feature_states, _ = edge_identity.get_all_feature_states() + feature_states = get_edge_identity_feature_states(edge_identity) # Then assert feature_states == [feature_state] @@ -519,8 +536,6 @@ def test_get_all_feature_states__post_v2_versioning_migration__returns_latest_ov feature_state: FeatureState, segment: Segment, segment_featurestate: FeatureState, - edge_identity_model: EdgeIdentity, - mocker: MockerFixture, django_assert_num_queries: DjangoAssertNumQueries, ) -> None: """ @@ -539,16 +554,18 @@ def test_get_all_feature_states__post_v2_versioning_migration__returns_latest_ov enable_v2_versioning(environment.id) - edge_identity_dynamo_wrapper_mock = mocker.patch( - "edge_api.identities.models.EdgeIdentity.dynamo_wrapper", + # and an identity belonging to the overridden segment + Condition.objects.create( + rule=SegmentRule.objects.create(segment=segment, type=SegmentRule.ALL_RULE), + property=MATCHING_TRAIT_KEY, + operator=EQUAL, + value=MATCHING_TRAIT_VALUE, ) - edge_identity_dynamo_wrapper_mock.get_segment_ids.return_value = [segment.id] + edge_identity = EdgeIdentity(_matching_identity_model(environment.api_key)) # When - with django_assert_num_queries(4): - feature_states, identity_override_feature_names = ( - edge_identity_model.get_all_feature_states() - ) + with django_assert_num_queries(8): + feature_states = get_edge_identity_feature_states(edge_identity) # Then assert len(feature_states) == 1 diff --git a/api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py b/api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py index 32bd84555fc9..eb195d313e84 100644 --- a/api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py +++ b/api/tests/unit/environments/dynamodb/wrappers/test_unit_dynamodb_identity_wrapper.py @@ -3,16 +3,12 @@ import pytest from boto3.dynamodb.conditions import Key -from boto3.dynamodb.types import Binary from botocore.exceptions import ClientError from django.core.exceptions import ObjectDoesNotExist -from flag_engine.segments.constants import IN, IS_SET from mypy_boto3_dynamodb.service_resource import Table -from pytest_django.fixtures import SettingsWrapper from pytest_mock import MockerFixture from rest_framework.exceptions import NotFound -from core.constants import INTEGER from edge_api.identities.search import ( IDENTIFIER_ATTRIBUTE, EdgeIdentitySearchData, @@ -27,22 +23,12 @@ from environments.identities.traits.constants import ( TRAIT_STRING_VALUE_MAX_LENGTH, ) -from environments.identities.traits.models import Trait -from features.models import Feature, FeatureSegment, FeatureState -from features.multivariate.models import ( - MultivariateFeatureOption, - MultivariateFeatureStateValue, -) -from segments.models import Condition, Segment, SegmentRule -from util.engine_models.identities.models import IdentityModel from util.mappers import ( - map_environment_to_compressed_environment_document, map_identity_to_identity_document, ) if typing.TYPE_CHECKING: - from environments.models import Environment - from projects.models import Project + pass def test_get_item_from_uuid__valid_uuid__calls_query_correctly(mocker): # type: ignore[no-untyped-def] @@ -316,128 +302,6 @@ def test_is_enabled__table_name_set__returns_true(settings, mocker): # type: ig mocked_boto3.resource.return_value.Table.assert_called_with(table_name) -def test_get_segment_ids__matching_segment_exists__returns_correct_ids( # type: ignore[no-untyped-def] - project, environment, identity, identity_matching_segment, mocker -): - # Given - two segments (one that matches the identity and one that does not) - Segment.objects.create(name="Non matching segment", project=project) - - identity_document = map_identity_to_identity_document(identity) - identity_uuid = identity_document["identity_uuid"] - - dynamo_identity_wrapper = DynamoIdentityWrapper() - mocked_get_item_from_uuid = mocker.patch.object( - dynamo_identity_wrapper, "get_item_from_uuid", return_value=identity_document - ) - - # When - segment_ids = dynamo_identity_wrapper.get_segment_ids(identity_uuid) # type: ignore[arg-type] - - # Then - assert segment_ids == [identity_matching_segment.id] - mocked_get_item_from_uuid.assert_called_with(identity_uuid) - - -def test_get_segment_ids__segment_with_feature_overrides__returns_correct_ids( - project: "Project", - environment: "Environment", - feature: "Feature", - identity: "Identity", - identity_matching_segment: "Segment", - mocker: "MockerFixture", -) -> None: - # Given - a segment with two feature overrides: - # one simple override and one with multivariate values - simple_feature_segment = FeatureSegment.objects.create( - feature=feature, - segment=identity_matching_segment, - environment=environment, - ) - FeatureState.objects.create( - feature=feature, - environment=environment, - feature_segment=simple_feature_segment, - enabled=True, - ) - - mv_feature = Feature.objects.create( - name="mv_feature", - project=project, - type="MULTIVARIATE", - ) - mv_option = MultivariateFeatureOption.objects.create( - feature=mv_feature, - default_percentage_allocation=30, - type="unicode", - string_value="variant_a", - ) - mv_feature_segment = FeatureSegment.objects.create( - feature=mv_feature, - segment=identity_matching_segment, - environment=environment, - ) - mv_feature_state = FeatureState.objects.create( - feature=mv_feature, - environment=environment, - feature_segment=mv_feature_segment, - enabled=True, - ) - MultivariateFeatureStateValue.objects.create( - feature_state=mv_feature_state, - multivariate_feature_option=mv_option, - percentage_allocation=30, - ) - - identity_document = map_identity_to_identity_document(identity) - identity_uuid = identity_document["identity_uuid"] - - dynamo_identity_wrapper = DynamoIdentityWrapper() - mocker.patch.object( - dynamo_identity_wrapper, "get_item_from_uuid", return_value=identity_document - ) - - # When - segment_ids = dynamo_identity_wrapper.get_segment_ids(identity_uuid) # type: ignore[arg-type] - - # Then - assert segment_ids == [identity_matching_segment.id] - - -def test_get_segment_ids__system_trait_backed_segment__returns_correct_ids( - project: "Project", - environment: "Environment", - identity: "Identity", - mocker: "MockerFixture", -) -> None: - # Given - two IS_SET segments: one keyed to a system trait the identity - # carries, one keyed to a system trait it does not - member_segment = Segment.objects.create(name="Cohort members", project=project) - rule = SegmentRule.objects.create(segment=member_segment, type=SegmentRule.ALL_RULE) - Condition.objects.create(rule=rule, operator=IS_SET, property="flagsmith_cohort_a") - other_segment = Segment.objects.create(name="Other cohort", project=project) - other_rule = SegmentRule.objects.create( - segment=other_segment, type=SegmentRule.ALL_RULE - ) - Condition.objects.create( - rule=other_rule, operator=IS_SET, property="flagsmith_cohort_b" - ) - - identity_document = map_identity_to_identity_document(identity) - identity_document["system_traits"] = {"flagsmith_cohort_a": True} - identity_uuid = identity_document["identity_uuid"] - - dynamo_identity_wrapper = DynamoIdentityWrapper() - mocker.patch.object( - dynamo_identity_wrapper, "get_item_from_uuid", return_value=identity_document - ) - - # When - segment_ids = dynamo_identity_wrapper.get_segment_ids(identity_uuid) # type: ignore[arg-type] - - # Then - assert segment_ids == [member_segment.id] - - def test_set_system_trait__oversized_string_value__raises() -> None: # Given wrapper = DynamoIdentityWrapper() @@ -452,138 +316,6 @@ def test_set_system_trait__oversized_string_value__raises() -> None: ) -def test_get_segment_ids__in_operator_with_integer_traits__returns_matching_segment( - project: "Project", environment: "Environment", mocker: "MockerFixture" -) -> None: - """ - Specific test to cover https://github.com/Flagsmith/flagsmith/issues/2602 - """ - # Given - trait_key = "trait_key" - - segment = Segment.objects.create(name="Test Segment", project=project) - parent_rule = SegmentRule.objects.create(segment=segment, type=SegmentRule.ALL_RULE) - child_rule = SegmentRule.objects.create(rule=parent_rule, type=SegmentRule.ANY_RULE) - Condition.objects.create( - property=trait_key, operator=IN, value="1,2,3,4", rule=child_rule - ) - - identity = Identity.objects.create(environment=environment, identifier="identifier") - Trait.objects.create( - trait_key=trait_key, integer_value=1, value_type=INTEGER, identity=identity - ) - - identity_document = map_identity_to_identity_document(identity) - identity_uuid = identity_document["identity_uuid"] - - dynamo_identity_wrapper = DynamoIdentityWrapper() - mocker.patch.object( - dynamo_identity_wrapper, "get_item_from_uuid", return_value=identity_document - ) - - # When - segment_ids = dynamo_identity_wrapper.get_segment_ids(identity_uuid) # type: ignore[arg-type] - - # Then - assert segment_ids == [segment.id] - - -def test_get_segment_ids__identity_does_not_exist__returns_empty_list( # type: ignore[no-untyped-def] - project, environment, identity, mocker -): - # Given - identity_document = map_identity_to_identity_document(identity) - dynamo_identity_wrapper = DynamoIdentityWrapper() - mocker.patch.object( - dynamo_identity_wrapper, "get_item_from_uuid", side_effect=ObjectDoesNotExist - ) - identity_uuid = identity_document["identity_uuid"] - - # When - segment_ids = dynamo_identity_wrapper.get_segment_ids(identity_uuid) # type: ignore[arg-type] - - # Then - assert segment_ids == [] - - -def test_get_segment_ids__no_arguments__raises_value_error(): # type: ignore[no-untyped-def] - # Given - dynamo_identity_wrapper = DynamoIdentityWrapper() - - # When / Then - with pytest.raises(ValueError): - dynamo_identity_wrapper.get_segment_ids() - - -def test_get_segment_ids__none_argument__raises_value_error(): # type: ignore[no-untyped-def] - # Given - dynamo_identity_wrapper = DynamoIdentityWrapper() - - # When / Then - with pytest.raises(ValueError): - dynamo_identity_wrapper.get_segment_ids(None) # type: ignore[arg-type] - - -def test_get_segment_ids__with_identity_model__returns_empty_list( # type: ignore[no-untyped-def] - identity, environment, mocker -): - # Given - identity_document = map_identity_to_identity_document(identity) - identity_model = IdentityModel.parse_obj(identity_document) - - dynamo_identity_wrapper = DynamoIdentityWrapper() - mocker.patch.object( - dynamo_identity_wrapper, "get_item_from_uuid", return_value=identity_document - ) - - # When - segment_ids = dynamo_identity_wrapper.get_segment_ids(identity_model=identity_model) - - # Then - assert segment_ids == [] - - -def test_get_segment_ids__compressed_environment_in_dynamo__returns_correct_segment_ids( - identity: "Identity", - identity_matching_segment: "Segment", - dynamodb_identity_wrapper: DynamoIdentityWrapper, - flagsmith_identities_table: Table, - flagsmith_environment_table: Table, - settings: "SettingsWrapper", -) -> None: - """Regression test for https://github.com/Flagsmith/flagsmith/issues/6912 - - Previously, get_segment_ids read the environment document from DynamoDB - and failed with a ValidationError when the document contained compressed - (gzipped Binary) `project` and `feature_states` fields. - """ - # Given - identity written to DynamoDB - identity_document = map_identity_to_identity_document(identity) - flagsmith_identities_table.put_item(Item=identity_document) - identity_uuid = str(identity_document["identity_uuid"]) - - # And - a compressed environment document in DynamoDB - settings.ENVIRONMENTS_TABLE_NAME_DYNAMO = flagsmith_environment_table.name - compressed_result = map_environment_to_compressed_environment_document( - identity.environment, - ) - flagsmith_environment_table.put_item(Item=compressed_result.document) - - # Verify the document actually has compressed Binary fields - stored = flagsmith_environment_table.get_item( - Key={"api_key": identity.environment.api_key}, - )["Item"] - assert stored.get("compressed") is True - assert isinstance(stored["project"], Binary) - assert isinstance(stored["feature_states"], Binary) - - # When - segment_ids = dynamodb_identity_wrapper.get_segment_ids(identity_uuid) - - # Then - assert segment_ids == [identity_matching_segment.id] - - def test_identity_wrapper__iter_all_items_paginated__returns_expected( identity: "Identity", mocker: "MockerFixture", diff --git a/api/tests/unit/environments/identities/test_unit_identities_models.py b/api/tests/unit/environments/identities/test_unit_identities_models.py index 05184155b1ca..b936bf3ebbf2 100644 --- a/api/tests/unit/environments/identities/test_unit_identities_models.py +++ b/api/tests/unit/environments/identities/test_unit_identities_models.py @@ -13,6 +13,7 @@ from environments.identities.models import Identity from environments.identities.traits.models import Trait from environments.models import Environment +from evaluation.services import get_identity_feature_states from features.models import ( Feature, FeatureSegment, @@ -106,7 +107,7 @@ def test_get_all_feature_states__multiple_identities_and_environments__returns_c ) # When - flags = identity_1.get_all_feature_states() + flags = get_identity_feature_states(identity_1) # Then # For identity_1 all items in a different environment should not appear. Identity @@ -249,7 +250,7 @@ def test_get_all_feature_states__matching_segment__returns_overridden_values( ) # When - feature_states = identity.get_all_feature_states() + feature_states = get_identity_feature_states(identity) # Then feature_flag_state = next( @@ -311,7 +312,7 @@ def test_get_all_feature_states__identity_not_matching_segment__returns_default_ ) # When - feature_states = identity.get_all_feature_states() + feature_states = get_identity_feature_states(identity) # Then feature_flag_state = next( @@ -367,7 +368,7 @@ def test_get_all_feature_states__matching_segment_with_integer_value__returns_ov ) # When - feature_states = identity.get_all_feature_states() + feature_states = get_identity_feature_states(identity) # Then feature_state = next(filter(lambda fs: fs.feature == remote_config, feature_states)) @@ -417,7 +418,7 @@ def test_get_all_feature_states__matching_segment_with_boolean_value__returns_ov ) # When - feature_states = identity.get_all_feature_states() + feature_states = get_identity_feature_states(identity) # Then feature_state = next(filter(lambda fs: fs.feature == remote_config, feature_states)) @@ -498,7 +499,7 @@ def test_get_all_feature_states__multiple_matching_segments__returns_highest_pri ) # When - we get all feature states for an identity - feature_states = identity.get_all_feature_states() + feature_states = get_identity_feature_states(identity) # Then - only the flag associated with the highest priority feature segment is returned assert len(feature_states) == 1 @@ -556,7 +557,7 @@ def test_get_all_feature_states__segment_override_updated__returns_new_value( overridden_value_2 = "overridden value 2" segment_feature_state.feature_state_value.string_value = overridden_value_2 segment_feature_state.feature_state_value.save() - feature_states = identity.get_all_feature_states() + feature_states = get_identity_feature_states(identity) # Then - the feature state value is correctly set to the newly updated feature segment value assert len(feature_states) == 1 @@ -617,7 +618,7 @@ def test_get_all_feature_states__traits_passed_manually__returns_segment_overrid ) # When - we get all feature states for an identity - feature_states = identity.get_all_feature_states(traits=[trait]) + feature_states = get_identity_feature_states(identity, traits=[trait]) # Then - the flag is returned with the correct state assert len(feature_states) == 1 @@ -825,7 +826,7 @@ def test_get_segments__matching_traits__returns_segment_with_expected_queries( # When # we get the matching segments for an identity - with django_assert_num_queries(7): + with django_assert_num_queries(8): segments = identity.get_segments() # Then @@ -896,7 +897,7 @@ def test_get_all_feature_states__null_version_exists__excludes_null_version( identity = Identity.objects.create(environment=environment, identifier="identity") # When - identity_feature_states = identity.get_all_feature_states() + identity_feature_states = get_identity_feature_states(identity) # Then assert len(identity_feature_states) == 1 @@ -992,7 +993,7 @@ def test_get_all_feature_states__hide_disabled_flags__returns_expected_flags( # ) # When # we get flags for the identity - identity_flags = identity.get_all_feature_states() + identity_flags = get_identity_feature_states(identity) # Then assert bool(identity_flags) == disabled_flag_returned @@ -1038,7 +1039,7 @@ def test_get_all_feature_states__multiple_versions__returns_latest_committed_ver not_live_feature_state.feature_state_value.save() # When - identity_feature_states = identity.get_all_feature_states() + identity_feature_states = get_identity_feature_states(identity) # Then identity_feature_state = next( @@ -1076,7 +1077,7 @@ def test_identity_get_all_feature_states__returns_identity_override__when_v2_fea ) # When - all_feature_states = identity.get_all_feature_states() + all_feature_states = get_identity_feature_states(identity) # Then assert len(all_feature_states) == 1 diff --git a/api/tests/unit/environments/identities/test_unit_identities_views.py b/api/tests/unit/environments/identities/test_unit_identities_views.py index 4d49ff843cec..679279c750e8 100644 --- a/api/tests/unit/environments/identities/test_unit_identities_views.py +++ b/api/tests/unit/environments/identities/test_unit_identities_views.py @@ -1373,10 +1373,10 @@ def test_identity_view_set__get_permissions__returns_expected_permissions(): # @pytest.mark.parametrize( ["use_replica", "is_new_identity", "num_queries"], [ - pytest.param(False, True, 12, id="default_database,new_identity"), - pytest.param(False, False, 7, id="default_database,existing_identity"), - pytest.param(True, True, 12, id="replica_database,new_identity"), - pytest.param(True, False, 9, id="replica_database,existing_identity"), + pytest.param(False, True, 14, id="default_database,new_identity"), + pytest.param(False, False, 9, id="default_database,existing_identity"), + pytest.param(True, True, 14, id="replica_database,new_identity"), + pytest.param(True, False, 11, id="replica_database,existing_identity"), ], ) def test_SDKIdentitiesDeprecated__given_identifier__retrieves_identity( diff --git a/api/tests/unit/evaluation/__init__.py b/api/tests/unit/evaluation/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/api/tests/unit/evaluation/test_unit_evaluation_mappers.py b/api/tests/unit/evaluation/test_unit_evaluation_mappers.py new file mode 100644 index 000000000000..2ea7fb7b6f5b --- /dev/null +++ b/api/tests/unit/evaluation/test_unit_evaluation_mappers.py @@ -0,0 +1,474 @@ +import pytest + +from edge_api.identities.models import EdgeIdentity +from environments.identities.models import Identity +from environments.identities.traits.models import Trait +from environments.models import Environment +from evaluation.mappers import ( + IDENTITY_OVERRIDES_SEGMENT_KEY, + IDENTITY_OVERRIDES_SEGMENT_NAME, + map_condition_to_segment_condition, + map_edge_identity_to_identity_context, + map_environment_to_evaluation_context, + map_feature_state_to_feature_context, + map_rule_to_segment_rule, + map_segment_to_segment_context, +) +from features.models import Feature, FeatureSegment, FeatureState +from features.multivariate.models import MultivariateFeatureStateValue +from segments.models import Condition, Segment, SegmentRule +from util.engine_models.identities.models import IdentityModel +from util.engine_models.identities.traits.models import TraitModel + + +def test_map_environment_to_evaluation_context__environment_default__populates_features( + identity: Identity, + feature: Feature, +) -> None: + # Given + feature_state = FeatureState.objects.get( + feature=feature, environment=identity.environment + ) + + # When + context = map_environment_to_evaluation_context( + environment=identity.environment, + identity=identity, + segments=identity.environment.get_segments_from_cache(), + ) + + # Then + assert context["features"] == { + feature.name: { + "key": str(feature_state.pk), + "name": feature.name, + "enabled": feature_state.enabled, + "value": feature_state.get_feature_state_value(), + "metadata": {"feature_state": feature_state}, + } + } + + +def test_map_environment_to_evaluation_context__transient_identity__omits_stored_traits( + environment: Environment, +) -> None: + # Given + # A transient identity is never saved, so it has no traits to read. + transient_identity = Identity(identifier="transient", environment=environment) + + # When + context = map_environment_to_evaluation_context( + environment=environment, + identity=transient_identity, + ) + + # Then + assert context["identity"] == { + "identifier": "transient", + "key": transient_identity.get_hash_key( + environment.use_identity_composite_key_for_hashing + ), + "traits": {}, + } + + +def test_map_environment_to_evaluation_context__segment_override__carries_segment_id( + identity: Identity, + feature: Feature, + identity_matching_segment: Segment, +) -> None: + # Given + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=identity_matching_segment, + environment=identity.environment, + priority=3, + ) + override = FeatureState.objects.create( + feature=feature, + environment=identity.environment, + feature_segment=feature_segment, + enabled=True, + ) + + # When + context = map_environment_to_evaluation_context( + environment=identity.environment, + identity=identity, + segments=identity.environment.get_segments_from_cache(), + ) + + # Then + segment_context = context["segments"][str(identity_matching_segment.pk)] + assert segment_context["metadata"] == { + "source": "segment", + "pk": identity_matching_segment.pk, + } + (override_context,) = segment_context["overrides"] + assert override_context["priority"] == 3 + assert override_context["metadata"] == {"feature_state": override} + + +def test_map_environment_to_evaluation_context__identity_override__returns_synthetic_segment( + identity: Identity, + feature: Feature, +) -> None: + # Given + override = FeatureState.objects.create( + identity=identity, + feature=feature, + environment=identity.environment, + enabled=True, + ) + + # When + context = map_environment_to_evaluation_context( + environment=identity.environment, + identity=identity, + segments=identity.environment.get_segments_from_cache(), + ) + + # Then + segment_context = context["segments"][IDENTITY_OVERRIDES_SEGMENT_KEY] + assert segment_context["name"] == IDENTITY_OVERRIDES_SEGMENT_NAME + assert segment_context["metadata"] == {"source": "identity_overrides"} + (override_context,) = segment_context["overrides"] + # No segment override may outrank an identity override. + assert override_context["priority"] == float("-inf") + assert override_context["metadata"] == {"feature_state": override} + + +def test_map_environment_to_evaluation_context__multivariate_feature__weights_variants_in_id_order( + identity: Identity, + multivariate_feature: Feature, +) -> None: + # Given + feature_state = FeatureState.objects.get( + feature=multivariate_feature, environment=identity.environment + ) + + # When + context = map_environment_to_evaluation_context( + environment=identity.environment, + identity=identity, + segments=identity.environment.get_segments_from_cache(), + ) + + # Then + # Core API allocates percentages in id order; the engine allocates in + # `priority` order, so the two only agree if priority follows id. + mv_values = MultivariateFeatureStateValue.objects.filter( + feature_state=feature_state + ).order_by("id") + assert context["features"][multivariate_feature.name]["variants"] == [ + { + "value": mv_value.multivariate_feature_option.value, + "weight": mv_value.percentage_allocation, + "priority": index, + } + for index, mv_value in enumerate(mv_values) + ] + + +@pytest.mark.parametrize( + "use_identity_composite_key_for_hashing", + [True, False], +) +def test_map_environment_to_evaluation_context__hashing_setting__sets_matching_identity_key( + identity: Identity, + use_identity_composite_key_for_hashing: bool, +) -> None: + """The identity key seeds multivariate allocation, so which one is used matters.""" + # Given + environment = identity.environment + environment.use_identity_composite_key_for_hashing = ( + use_identity_composite_key_for_hashing + ) + environment.save() + + # When + context = map_environment_to_evaluation_context( + environment=environment, + identity=identity, + ) + + # Then + identity_context = context["identity"] + assert identity_context + assert identity_context["key"] == ( + identity.composite_key + if use_identity_composite_key_for_hashing + else str(identity.pk) + ) + + +@pytest.mark.parametrize( + ["mv_hashing_salt", "expected_key"], + [ + pytest.param(None, "id", id="no_salt"), + pytest.param(999, "999", id="salt_set"), + ], +) +def test_map_feature_state_to_feature_context__multivariate_feature__keys_on_hashing_seed( + environment: Environment, + multivariate_feature: Feature, + mv_hashing_salt: int | None, + expected_key: str, +) -> None: + """The context key seeds multivariate allocation. + + It has to be the lineage constant (#7913), so that recreating a feature + state does not move enrolled identities to another variant. + """ + # Given + feature_state = FeatureState.objects.get( + environment=environment, + feature=multivariate_feature, + identity=None, + feature_segment=None, + ) + feature_state.mv_hashing_salt = mv_hashing_salt + + # When + feature_context = map_feature_state_to_feature_context(feature_state) + + # Then + assert feature_context["key"] == ( + str(feature_state.id) if expected_key == "id" else expected_key + ) + + +def test_map_environment_to_evaluation_context__no_identity__returns_environment_only( + environment: Environment, +) -> None: + # Given / When + context = map_environment_to_evaluation_context(environment=environment) + + # Then + assert context == { + "environment": { + "key": environment.api_key, + "name": environment.name, + }, + "features": {}, + } + + +def test_map_environment_to_evaluation_context__with_identity__returns_identity_context( + environment: Environment, + identity: Identity, +) -> None: + # Given / When + context = map_environment_to_evaluation_context( + environment=environment, + identity=identity, + ) + + # Then + assert context == { + "environment": { + "key": environment.api_key, + "name": environment.name, + }, + "identity": { + "identifier": identity.identifier, + "key": identity.get_hash_key( + environment.use_identity_composite_key_for_hashing + ), + "traits": {}, + }, + "features": {}, + } + + +def test_map_environment_to_evaluation_context__with_explicit_traits__returns_given_traits( + environment: Environment, + identity: Identity, + trait: Trait, +) -> None: + # Given / When + context = map_environment_to_evaluation_context( + environment=environment, + identity=identity, + traits=[trait], + ) + + # Then + assert context == { + "environment": { + "key": environment.api_key, + "name": environment.name, + }, + "identity": { + "identifier": identity.identifier, + "key": identity.get_hash_key( + environment.use_identity_composite_key_for_hashing + ), + "traits": {trait.trait_key: trait.trait_value}, + }, + "features": {}, + } + + +def test_map_environment_to_evaluation_context__no_explicit_traits__returns_identity_traits( + environment: Environment, + identity: Identity, + trait: Trait, +) -> None: + # Given / When + context = map_environment_to_evaluation_context( + environment=environment, + identity=identity, + ) + + # Then + assert context == { + "environment": { + "key": environment.api_key, + "name": environment.name, + }, + "identity": { + "identifier": identity.identifier, + "key": identity.get_hash_key( + environment.use_identity_composite_key_for_hashing + ), + "traits": {trait.trait_key: trait.trait_value}, + }, + "features": {}, + } + + +def test_map_environment_to_evaluation_context__with_segments__returns_segment_contexts( + environment: Environment, + identity_matching_segment: Segment, +) -> None: + # Given / When + context = map_environment_to_evaluation_context( + environment=environment, + segments=[identity_matching_segment], + ) + + # Then + segment_key = str(identity_matching_segment.pk) + assert context == { + "environment": { + "key": environment.api_key, + "name": environment.name, + }, + "segments": { + segment_key: map_segment_to_segment_context(identity_matching_segment), + }, + "features": {}, + } + + +def test_map_segment_to_segment_context__segment_with_rule__returns_expected( + identity_matching_segment: Segment, +) -> None: + # Given + condition = Condition.objects.get( + rule__segment=identity_matching_segment, + ) + + # When + result = map_segment_to_segment_context(identity_matching_segment) + + # Then + assert result == { + "key": str(identity_matching_segment.pk), + "name": identity_matching_segment.name, + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": condition.property, + "operator": condition.operator, + "value": condition.value, + }, + ], + "rules": [], + }, + ], + "metadata": {"source": "segment", "pk": identity_matching_segment.pk}, + } + + +def test_map_rule_to_segment_rule__with_nested_rule__returns_expected( + segment_rule: SegmentRule, + identity_matching_segment: Segment, +) -> None: + # Given + matching_rule = SegmentRule.objects.get(segment=identity_matching_segment) + matching_rule.rules.add(segment_rule) + condition = Condition.objects.get(rule=matching_rule) + + # When + result = map_rule_to_segment_rule(matching_rule) + + # Then + assert result == { + "type": "ALL", + "conditions": [ + { + "property": condition.property, + "operator": condition.operator, + "value": condition.value, + }, + ], + "rules": [ + { + "type": "ALL", + "conditions": [], + "rules": [], + }, + ], + } + + +def test_map_condition_to_segment_condition__valid_condition__returns_expected( + identity_matching_segment: Segment, +) -> None: + # Given + condition = Condition.objects.get( + rule__segment=identity_matching_segment, + ) + + # When + result = map_condition_to_segment_condition(condition) + + # Then + assert result == { + "property": condition.property, + "operator": condition.operator, + "value": condition.value, + } + + +def test_map_edge_identity_to_identity_context__system_traits__merged_with_system_winning( + environment: Environment, +) -> None: + # Given + edge_identity = EdgeIdentity( + IdentityModel( + identifier="identity", + environment_api_key=environment.api_key, + identity_traits=[ + TraitModel(trait_key="owned-by-user", trait_value="user value"), + TraitModel(trait_key="clashing", trait_value="user value"), + ], + system_traits={"clashing": "system value"}, + ) + ) + + # When + identity_context = map_edge_identity_to_identity_context( + edge_identity, environment=environment + ) + + # Then + assert identity_context["traits"] == { + "owned-by-user": "user value", + # System-owned traits are not user data, so they win a key clash. + "clashing": "system value", + } diff --git a/api/tests/unit/evaluation/test_unit_evaluation_services.py b/api/tests/unit/evaluation/test_unit_evaluation_services.py new file mode 100644 index 000000000000..9a3029e65b82 --- /dev/null +++ b/api/tests/unit/evaluation/test_unit_evaluation_services.py @@ -0,0 +1,481 @@ +import pytest +from flag_engine.segments.constants import EQUAL, IN, IS_SET +from pytest_lazy_fixtures import lf as lazy_fixture + +from edge_api.identities.models import EdgeIdentity +from environments.identities.models import Identity +from environments.identities.traits.models import Trait +from environments.models import Environment +from evaluation.services import ( + evaluate_identity, + get_edge_identity_feature_states, + get_edge_identity_segments, +) +from features.constants import CONTROL_VARIANT_KEY +from features.feature_types import MULTIVARIATE +from features.models import Feature, FeatureSegment, FeatureState +from features.multivariate.models import ( + MultivariateFeatureOption, + MultivariateFeatureStateValue, +) +from features.value_types import INTEGER, STRING +from projects.models import Project +from segments.models import Condition, Segment, SegmentRule +from util.engine_models.features.models import FeatureModel, FeatureStateModel +from util.engine_models.identities.models import IdentityFeaturesList, IdentityModel +from util.engine_models.identities.traits.models import TraitModel +from util.mappers import map_identity_to_identity_document + + +@pytest.fixture() +def control_value() -> str: + return "control" + + +@pytest.fixture() +def mv_hashing_salt() -> int: + return 1 + + +@pytest.fixture() +def hashing_environment_api_key() -> str: + return "test-environment-key" + + +@pytest.fixture() +def hashing_environment( + environment: Environment, + hashing_environment_api_key: str, +) -> Environment: + """An environment whose identities bucket predictably.""" + environment.api_key = hashing_environment_api_key + environment.use_identity_composite_key_for_hashing = True + environment.save() + return environment + + +def test_evaluate_identity__identity_and_segment_override__identity_override_wins( + identity: Identity, + feature: Feature, + identity_matching_segment: Segment, +) -> None: + # Given + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=identity_matching_segment, + environment=identity.environment, + # A priority high enough to beat every other segment override, but not + # an identity override. + priority=0, + ) + segment_override = FeatureState.objects.create( + feature=feature, + environment=identity.environment, + feature_segment=feature_segment, + enabled=True, + ) + segment_override.feature_state_value.string_value = "segment" + segment_override.feature_state_value.save() + + identity_override = FeatureState.objects.create( + identity=identity, + feature=feature, + environment=identity.environment, + enabled=True, + ) + identity_override.feature_state_value.string_value = "identity" + identity_override.feature_state_value.save() + + # When + result, _ = evaluate_identity(identity) + + # Then + flag = result["flags"][feature.name] + assert flag["value"] == "identity" + assert flag["metadata"]["feature_state"] == identity_override + + +def test_evaluate_identity__segment_overrides__lowest_priority_wins( + identity: Identity, + feature: Feature, + identity_matching_segment: Segment, + project: Project, + trait: Trait, +) -> None: + # Given + # A second segment the identity also matches, overriding the same feature. + other_segment = Segment.objects.create(name="Everyone", project=project) + Condition.objects.create( + rule=SegmentRule.objects.create( + segment=other_segment, type=SegmentRule.ALL_RULE + ), + property=trait.trait_key, + operator=EQUAL, + value=trait.trait_value, + ) + + for segment, priority, value in ( + (identity_matching_segment, 1, "winner"), + (other_segment, 2, "loser"), + ): + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=identity.environment, + priority=priority, + ) + override = FeatureState.objects.create( + feature=feature, + environment=identity.environment, + feature_segment=feature_segment, + ) + override.feature_state_value.string_value = value + override.feature_state_value.save() + + # When + result, _ = evaluate_identity(identity) + + # Then + assert result["flags"][feature.name]["value"] == "winner" + + +def test_evaluate_identity__multivariate_feature__buckets_as_before_the_engine( + hashing_environment: Environment, + mv_hashing_salt: int, + project: Project, +) -> None: + """The expectations are derived from Core API's allocation as it stood before flag-engine took over.""" + # Given + # ten equal variants, so the variant an identity gets names the decile its + # hash fell in + expected_variant_by_identifier = { + "identity-0": "variant-3", + "identity-1": "variant-0", + "identity-2": "variant-2", + "identity-3": "variant-5", + "identity-4": "variant-8", + "identity-5": "variant-5", + "identity-6": "variant-1", + "identity-7": "variant-2", + "identity-8": "variant-2", + "identity-9": "variant-1", + } + feature = Feature.objects.create( + name="decile_feature", + project=project, + type=MULTIVARIATE, + initial_value="control", + ) + for index in range(10): + MultivariateFeatureOption.objects.create( + feature=feature, + default_percentage_allocation=10, + type=STRING, + string_value=f"variant-{index}-value", + key=f"variant-{index}", + ) + + feature_state = FeatureState.objects.get( + environment=hashing_environment, + feature=feature, + identity=None, + feature_segment=None, + ) + feature_state.mv_hashing_salt = mv_hashing_salt + feature_state.save() + + # When + variant_by_identifier = { + identifier: evaluate_identity( + Identity.objects.create( + identifier=identifier, environment=hashing_environment + ) + ).result["flags"][feature.name]["variant"] + for identifier in expected_variant_by_identifier + } + + # Then + assert variant_by_identifier == expected_variant_by_identifier + + +def test_evaluate_identity__multivariate_feature__returns_variant_key( + identity: Identity, + multivariate_feature: Feature, +) -> None: + # Given + feature_state = FeatureState.objects.get( + feature=multivariate_feature, environment=identity.environment + ) + for index, mv_value in enumerate( + MultivariateFeatureStateValue.objects.filter( + feature_state=feature_state + ).order_by("id") + ): + mv_value.multivariate_feature_option.key = f"variant-{index}" + mv_value.multivariate_feature_option.save() + + # When + result, _ = evaluate_identity(identity) + + # Then + flag = result["flags"][multivariate_feature.name] + # Either a named variant or the control bucket + assert flag["variant"] in {"control", "variant-0", "variant-1", "variant-2"} + + +@pytest.mark.parametrize( + ["identifier", "expected_variant", "expected_value"], + ( + pytest.param("identity-1", "variant-1", "variant-1-value", id="first_band"), + pytest.param("identity-2", "variant-2", "variant-2-value", id="second_band"), + pytest.param( + "identity-4", + CONTROL_VARIANT_KEY, + lazy_fixture("control_value"), + id="unallocated_falls_through", + ), + ), +) +def test_evaluate_identity__multivariate_feature__allocates_variants_in_order( + hashing_environment: Environment, + mv_hashing_salt: int, + multivariate_feature: Feature, + identifier: str, + expected_variant: str, + expected_value: str, +) -> None: + # Given + feature_state = FeatureState.objects.get( + environment=hashing_environment, + feature=multivariate_feature, + identity=None, + feature_segment=None, + ) + feature_state.mv_hashing_salt = mv_hashing_salt + feature_state.save() + + # Two variants taking 20% and 30%, leaving half the range to the control. + # The fixture gives two of its options the same value, so name them apart. + for index, (mv_value, allocation) in enumerate( + zip( + feature_state.multivariate_feature_state_values.order_by("id"), + (20, 30, 0), + ) + ): + mv_value.percentage_allocation = allocation + mv_value.save() + option = mv_value.multivariate_feature_option + option.key = f"variant-{index + 1}" + option.string_value = f"variant-{index + 1}-value" + option.save() + + identity = Identity.objects.create( + identifier=identifier, environment=hashing_environment + ) + + # When + flag = evaluate_identity(identity).result["flags"][multivariate_feature.name] + + # Then + assert flag["variant"] == expected_variant + assert flag["value"] == expected_value + + +def test_get_edge_identity_feature_states__segment_and_identity_override__identity_override_wins( + environment: Environment, + feature: Feature, + identity_matching_segment: Segment, + trait: Trait, +) -> None: + # Given + # a segment override the identity matches + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=identity_matching_segment, + environment=environment, + priority=0, + ) + segment_override = FeatureState.objects.create( + feature=feature, + environment=environment, + feature_segment=feature_segment, + enabled=True, + ) + segment_override.feature_state_value.string_value = "segment" + segment_override.feature_state_value.save() + + # and an identity override, stored against the identity in DynamoDB + edge_identity = EdgeIdentity( + IdentityModel( + identifier="identity", + environment_api_key=environment.api_key, + identity_traits=[ + TraitModel(trait_key=trait.trait_key, trait_value=trait.trait_value) + ], + identity_features=IdentityFeaturesList( + [ + FeatureStateModel( + django_id=1, + feature=FeatureModel( + id=feature.id, name=feature.name, type=feature.type + ), + enabled=True, + feature_state_value="identity", + ) + ] + ), + ) + ) + + # When + feature_states = get_edge_identity_feature_states(edge_identity) + + # Then + (feature_state,) = [ + feature_state + for feature_state in feature_states + if feature_state.feature.name == feature.name + ] + assert feature_state.feature_state_value == "identity" + + +def test_get_edge_identity_segments__matching_segment_exists__returns_matching_only( + project: Project, + environment: Environment, + identity: Identity, + identity_matching_segment: Segment, +) -> None: + # Given - two segments (one that matches the identity and one that does not) + Segment.objects.create(name="Non matching segment", project=project) + + edge_identity = EdgeIdentity.from_identity_document( + map_identity_to_identity_document(identity) + ) + + # When + segments = get_edge_identity_segments(edge_identity) + + # Then + assert segments == [identity_matching_segment] + + +def test_get_edge_identity_segments__segment_with_feature_overrides__returns_matching_only( + project: Project, + environment: Environment, + feature: Feature, + identity: Identity, + identity_matching_segment: Segment, +) -> None: + # Given - a segment with two feature overrides: + # one simple override and one with multivariate values + simple_feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=identity_matching_segment, + environment=environment, + ) + FeatureState.objects.create( + feature=feature, + environment=environment, + feature_segment=simple_feature_segment, + enabled=True, + ) + + mv_feature = Feature.objects.create( + name="mv_feature", + project=project, + type="MULTIVARIATE", + ) + mv_option = MultivariateFeatureOption.objects.create( + feature=mv_feature, + default_percentage_allocation=30, + type="unicode", + string_value="variant_a", + ) + mv_feature_segment = FeatureSegment.objects.create( + feature=mv_feature, + segment=identity_matching_segment, + environment=environment, + ) + mv_feature_state = FeatureState.objects.create( + feature=mv_feature, + environment=environment, + feature_segment=mv_feature_segment, + enabled=True, + ) + MultivariateFeatureStateValue.objects.create( + feature_state=mv_feature_state, + multivariate_feature_option=mv_option, + percentage_allocation=30, + ) + + edge_identity = EdgeIdentity.from_identity_document( + map_identity_to_identity_document(identity) + ) + + # When + segments = get_edge_identity_segments(edge_identity) + + # Then + assert segments == [identity_matching_segment] + + +def test_get_edge_identity_segments__system_trait_backed_segment__returns_matching_only( + project: Project, + environment: Environment, + identity: Identity, +) -> None: + # Given - two IS_SET segments: one keyed to a system trait the identity + # carries, one keyed to a system trait it does not + member_segment = Segment.objects.create(name="Cohort members", project=project) + rule = SegmentRule.objects.create(segment=member_segment, type=SegmentRule.ALL_RULE) + Condition.objects.create(rule=rule, operator=IS_SET, property="flagsmith_cohort_a") + other_segment = Segment.objects.create(name="Other cohort", project=project) + other_rule = SegmentRule.objects.create( + segment=other_segment, type=SegmentRule.ALL_RULE + ) + Condition.objects.create( + rule=other_rule, operator=IS_SET, property="flagsmith_cohort_b" + ) + + identity_document = map_identity_to_identity_document(identity) + identity_document["system_traits"] = {"flagsmith_cohort_a": True} + edge_identity = EdgeIdentity.from_identity_document(identity_document) + + # When + segments = get_edge_identity_segments(edge_identity) + + # Then + assert segments == [member_segment] + + +def test_get_edge_identity_segments__in_operator_with_integer_traits__returns_matching_only( + project: Project, + environment: Environment, +) -> None: + """ + Specific test to cover https://github.com/Flagsmith/flagsmith/issues/2602 + """ + # Given + trait_key = "trait_key" + + segment = Segment.objects.create(name="Test Segment", project=project) + parent_rule = SegmentRule.objects.create(segment=segment, type=SegmentRule.ALL_RULE) + child_rule = SegmentRule.objects.create(rule=parent_rule, type=SegmentRule.ANY_RULE) + Condition.objects.create( + property=trait_key, operator=IN, value="1,2,3,4", rule=child_rule + ) + + identity = Identity.objects.create(environment=environment, identifier="identifier") + Trait.objects.create( + trait_key=trait_key, integer_value=1, value_type=INTEGER, identity=identity + ) + + edge_identity = EdgeIdentity.from_identity_document( + map_identity_to_identity_document(identity) + ) + + # When + segments = get_edge_identity_segments(edge_identity) + + # Then + assert segments == [segment] diff --git a/api/tests/unit/experimentation/test_services.py b/api/tests/unit/experimentation/test_services.py index 1932355a1381..446e4dad3c38 100644 --- a/api/tests/unit/experimentation/test_services.py +++ b/api/tests/unit/experimentation/test_services.py @@ -5,7 +5,6 @@ import pytest from django.db import IntegrityError, connection -from django.db.models import Q from django.test.utils import CaptureQueriesContext from flag_engine.segments.constants import EQUAL, PERCENTAGE_SPLIT from prometheus_client import REGISTRY @@ -23,6 +22,7 @@ from environments.identities.models import Identity from environments.identities.traits.models import Trait from environments.models import Environment +from evaluation.services import get_identity_feature_states from experimentation import services from experimentation.constants import MAX_AUDIENCE_SEGMENTS from experimentation.dataclasses import ( @@ -66,6 +66,7 @@ from organisations.models import Organisation from projects.models import Project from segments.models import Condition, Segment, SegmentRule +from tests.types import VariantAssignmentFixture from tests.unit.experimentation.conftest import RolloutSpecFactory from users.models import FFAdminUser from util.mappers import map_environment_to_environment_document @@ -2569,9 +2570,15 @@ def test_apply_experiment_rollout__reapplied_under_v2__keeps_variant_assignment( multivariate_feature: Feature, multivariate_options: list[MultivariateFeatureOption], admin_user: FFAdminUser, + variant_assignment: VariantAssignmentFixture, ) -> None: # Given a running experiment whose rollout splits two variants 50/50 option_a, option_b, _ = multivariate_options + # The fixture derives an option's value from its percentage, so two of them + # share a value. Key them so a variant identifies which option won. + for index, option in enumerate(multivariate_options): + option.key = f"variant-{index}" + option.save() experiment = Experiment.objects.create( environment=environment_v2_versioning, feature=multivariate_feature, @@ -2590,43 +2597,28 @@ def test_apply_experiment_rollout__reapplied_under_v2__keeps_variant_assignment( ], author=AuthorData(user=admin_user), ) - identity_hash_keys = [f"identity-{i}" for i in range(50)] - - def variant_assignment() -> dict[str, int]: - override = ( - FeatureState.objects.get_live_feature_states( - environment=experiment.environment, - additional_filters=Q( - feature_segment__segment=experiment.rollout_segment, - identity__isnull=True, - ), - feature_id=experiment.feature_id, - ) - .prefetch_related( - "multivariate_feature_state_values__multivariate_feature_option" - ) - .latest("id") + # and identities, some of which the rollout's percentage split enrols + identities = [ + Identity.objects.create( + identifier=f"identity-{i}", environment=environment_v2_versioning ) - assignment: dict[str, int] = {} - for key in identity_hash_keys: - option = override.get_multivariate_feature_state_value(key) - # The 50/50 split allocates 100%, so every identity lands on an option. - assert isinstance(option, MultivariateFeatureOption) - assignment[key] = option.id - return assignment + for i in range(50) + ] # When the rollout is applied, then re-applied unchanged (e.g. tuned while # the experiment is running) services.apply_experiment_rollout(experiment, spec) experiment.refresh_from_db() - before = variant_assignment() + before = variant_assignment(identities, multivariate_feature.name) services.apply_experiment_rollout(experiment, spec) - after = variant_assignment() + after = variant_assignment(identities, multivariate_feature.name) # Then every already-enrolled identity keeps the variant it was first # assigned; tuning the rollout must not re-randomise the split. assert before == after + # and the split is not trivially one-sided, so the above means something + assert len(set(before.values())) > 1 def _verification_count(result: str) -> float: @@ -3507,11 +3499,11 @@ def _identity_flag_value( feature segment it came from (``None`` for the environment default).""" (feature_state,) = [ feature_state - for feature_state in identity.get_all_feature_states() + for feature_state in get_identity_feature_states(identity) if feature_state.feature_id == feature.id ] return ( - feature_state.get_feature_state_value(identity=identity), + feature_state.evaluated_value, ( feature_state.feature_segment.segment_id if feature_state.feature_segment diff --git a/api/tests/unit/features/test_unit_features_models.py b/api/tests/unit/features/test_unit_features_models.py index 40325673c236..ea1a826b8167 100644 --- a/api/tests/unit/features/test_unit_features_models.py +++ b/api/tests/unit/features/test_unit_features_models.py @@ -611,127 +611,27 @@ def test_feature_state_type__feature_segment_state__returns_feature_segment( assert result == FEATURE_SEGMENT -@pytest.mark.parametrize("hashed_percentage", (0.0, 30.0, 50.0, 80.0, 99.9999)) -@mock.patch("features.models.get_hashed_percentage_for_object_ids") -def test_get_multivariate_feature_state_value__with_identity__returns_correct_value( # type: ignore[no-untyped-def] - mock_get_hashed_percentage, - hashed_percentage, - multivariate_feature, - environment, - identity, -): - # Given - mock_get_hashed_percentage.return_value = hashed_percentage - feature_state = FeatureState.objects.get( - environment=environment, - feature=multivariate_feature, - identity=None, - feature_segment=None, - ) - - # When - multivariate_value = feature_state.get_multivariate_feature_state_value( - identity_hash_key=identity.get_hash_key() - ) - - # Then - # we get a multivariate value - assert multivariate_value - - # and that value is not the control (since the fixture includes values that span - # the entire 100%) - assert multivariate_value.value != multivariate_value.initial_value - - -@mock.patch("features.models.get_hashed_percentage_for_object_ids") -def test_get_multivariate_feature_state_value__no_mv_hashing_salt__seeds_hash_with_id( # type: ignore[no-untyped-def] - mock_get_hashed_percentage, - multivariate_feature, - environment, - identity, -): - # Given - mock_get_hashed_percentage.return_value = 0.0 - feature_state = FeatureState.objects.get( - environment=environment, - feature=multivariate_feature, - identity=None, - feature_segment=None, - ) - assert feature_state.mv_hashing_salt is None - identity_hash_key = identity.get_hash_key() - - # When - feature_state.get_multivariate_feature_state_value( - identity_hash_key=identity_hash_key - ) - - # Then the feature state id seeds the hash - mock_get_hashed_percentage.assert_called_once_with( - [feature_state.id, identity_hash_key] - ) - - -@mock.patch("features.models.get_hashed_percentage_for_object_ids") -def test_get_multivariate_feature_state_value__mv_hashing_salt_set__seeds_hash_with_salt( # type: ignore[no-untyped-def] - mock_get_hashed_percentage, - multivariate_feature, - environment, - identity, -): - # Given - mock_get_hashed_percentage.return_value = 0.0 - feature_state = FeatureState.objects.get( - environment=environment, - feature=multivariate_feature, - identity=None, - feature_segment=None, - ) - feature_state.mv_hashing_salt = 999 - identity_hash_key = identity.get_hash_key() - - # When - feature_state.get_multivariate_feature_state_value( - identity_hash_key=identity_hash_key - ) - - # Then the salt seeds the hash instead of the feature state id - mock_get_hashed_percentage.assert_called_once_with([999, identity_hash_key]) - - -def test_feature_state_clone__multivariate_feature__keeps_variant_bucketing_stable( +def test_feature_state_clone__multivariate_feature__carries_source_id_as_salt( multivariate_feature: Feature, environment: Environment, environment_two: Environment, ) -> None: - # Given the environment-default feature state for a multivariate feature, and - # the variant each of a range of identities is currently bucketed into + # Given feature_state = FeatureState.objects.get( environment=environment, feature=multivariate_feature, identity=None, feature_segment=None, ) - identity_hash_keys = [f"identity-{i}" for i in range(50)] - original_assignment = { - key: feature_state.get_multivariate_feature_state_value(key).id - for key in identity_hash_keys - } + assert feature_state.mv_hashing_salt is None - # When the feature state is recreated by cloning it (e.g. publishing a new - # version or editing multivariate weights under v2 versioning) + # When cloned_feature_state = feature_state.clone(env=environment_two, as_draft=True) - # Then the clone keeps the original feature state's id as its bucketing salt + # Then assert cloned_feature_state.id != feature_state.id assert cloned_feature_state.mv_hashing_salt == feature_state.id - - # and every identity stays in the same variant as before - cloned_assignment = { - key: cloned_feature_state.get_multivariate_feature_state_value(key).id - for key in identity_hash_keys - } - assert cloned_assignment == original_assignment + assert cloned_feature_state.mv_hashing_seed == feature_state.mv_hashing_seed def test_feature_state_clone__existing_mv_hashing_salt__is_preserved( @@ -852,61 +752,6 @@ def test_feature_state_create__new_segment_override_under_v2__no_salt_inherited( assert feature_state.mv_hashing_salt is None -@mock.patch.object(FeatureState, "get_multivariate_feature_state_value") -def test_get_feature_state_value__multivariate_feature__returns_mv_value( # type: ignore[no-untyped-def] - mock_get_mv_feature_state_value, environment, multivariate_feature, identity -): - # Given - value = "value" - mock_mv_feature_state_value = mock.MagicMock(value=value) - mock_get_mv_feature_state_value.return_value = mock_mv_feature_state_value - - environment.use_identity_composite_key_for_hashing = False - environment.save() - - feature_state = FeatureState.objects.get( - environment=environment, - feature=multivariate_feature, - identity=None, - feature_segment=None, - ) - - # When - feature_state_value = feature_state.get_feature_state_value(identity=identity) - - # Then - # the correct value is returned - assert feature_state_value == value - # and the correct call is made to get the multivariate feature state value - mock_get_mv_feature_state_value.assert_called_once_with(str(identity.id)) - - -@mock.patch.object(FeatureState, "get_multivariate_feature_state_value") -def test_get_feature_state_value__multivariate_v2_evaluation__uses_composite_key( # type: ignore[no-untyped-def] - mock_get_mv_feature_state_value, environment, multivariate_feature, identity -): - # Given - value = "value" - mock_mv_feature_state_value = mock.MagicMock(value=value) - mock_get_mv_feature_state_value.return_value = mock_mv_feature_state_value - - feature_state = FeatureState.objects.get( - environment=environment, - feature=multivariate_feature, - identity=None, - feature_segment=None, - ) - - # When - feature_state_value = feature_state.get_feature_state_value(identity=identity) - - # Then - # the correct value is returned - assert feature_state_value == value - # and the correct call is made to get the multivariate feature state value - mock_get_mv_feature_state_value.assert_called_once_with(identity.composite_key) - - @pytest.mark.parametrize( "feature_state_version_generator", ( diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index f47d3951d262..484d5dec3ba8 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -20,6 +20,7 @@ from django.forms import model_to_dict from django.urls import reverse from django.utils import timezone +from flag_engine.segments.constants import EQUAL from freezegun import freeze_time from pytest_django import DjangoAssertNumQueries from pytest_django.fixtures import SettingsWrapper @@ -60,7 +61,7 @@ from projects.code_references.models import ScannedCodeReferences, VCSRepository from projects.models import Project, UserProjectPermission from projects.tags.models import Tag -from segments.models import Segment +from segments.models import Condition, Segment, SegmentRule from tests.types import ( WithEnvironmentPermissionsCallable, WithProjectPermissionsCallable, @@ -704,6 +705,47 @@ def test_get_flags__environment_with_overrides__returns_environment_default( ) +def test_get_flags__segment_matching_without_identity__returns_segment_override( + api_client: APIClient, + environment: Environment, + project: Project, +) -> None: + """A segment that does not depend on an identity applies without one. + + Which is how an SDK evaluating the environment document locally has + always read it. + """ + # Given + feature = Feature.objects.create( + name="Test feature", project=project, initial_value="environment" + ) + segment = Segment.objects.create(name="This environment", project=project) + Condition.objects.create( + rule=SegmentRule.objects.create(segment=segment, type=SegmentRule.ALL_RULE), + property="$.environment.name", + operator=EQUAL, + value=environment.name, + ) + feature_segment = FeatureSegment.objects.create( + segment=segment, feature=feature, environment=environment + ) + segment_override = FeatureState.objects.create( + feature=feature, feature_segment=feature_segment, environment=environment + ) + segment_override.feature_state_value.string_value = "segment" + segment_override.feature_state_value.save() + + api_client.credentials(HTTP_X_ENVIRONMENT_KEY=environment.api_key) + + # When + response = api_client.get(reverse("api-v1:flags")) + + # Then + assert response.status_code == status.HTTP_200_OK + (flag,) = [flag for flag in response.json() if flag["feature"]["id"] == feature.id] + assert flag["feature_state_value"] == "segment" + + @pytest.mark.parametrize("cache_flags_seconds", [0, 30]) def test_sdk_feature_states_get__no_identifier__returns_feature_list( api_client: APIClient, diff --git a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py index f190e7d86741..fcf7ba5f68ea 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py @@ -32,6 +32,7 @@ ) from projects.models import Project from segments.models import Segment +from tests.types import VariantAssignmentFixture from users.models import FFAdminUser @@ -1042,12 +1043,18 @@ def test_update_flag__v2_versioning_multivariate_weight_increase__keeps_enrolled multivariate_feature: Feature, multivariate_options: list[MultivariateFeatureOption], admin_user: FFAdminUser, + variant_assignment: VariantAssignmentFixture, ) -> None: # Given a multivariate feature split 50/50 between two variants, and the # variant each of a range of identities is bucketed into author = AuthorData(user=admin_user) option_a, option_b, option_c = multivariate_options - feature_state = update_flag( + # The fixture derives an option's value from its percentage, so two of them + # share a value. Key them so a variant identifies which option won. + for index, option in enumerate(multivariate_options): + option.key = f"variant-{index}" + option.save() + update_flag( environment_v2_versioning, multivariate_feature, FlagChangeSet( @@ -1062,14 +1069,16 @@ def test_update_flag__v2_versioning_multivariate_weight_increase__keeps_enrolled ], ), ) - identity_hash_keys = [f"identity-{i}" for i in range(100)] - original_assignment = { - key: feature_state.get_multivariate_feature_state_value(key).pk - for key in identity_hash_keys - } + identities = [ + Identity.objects.create( + identifier=f"identity-{i}", environment=environment_v2_versioning + ) + for i in range(100) + ] + original_assignment = variant_assignment(identities, multivariate_feature.name) # When the first variant's allocation is increased to 60/40 - new_feature_state = update_flag( + update_flag( environment_v2_versioning, multivariate_feature, FlagChangeSet( @@ -1087,20 +1096,17 @@ def test_update_flag__v2_versioning_multivariate_weight_increase__keeps_enrolled # Then identities already in the grown variant stay in it, and the only # movement is from the shrunk variant into the grown one - new_assignment = { - key: new_feature_state.get_multivariate_feature_state_value(key).pk - for key in identity_hash_keys - } + new_assignment = variant_assignment(identities, multivariate_feature.name) movers = { key - for key in identity_hash_keys + for key in original_assignment if new_assignment[key] != original_assignment[key] } assert movers - assert all(original_assignment[key] == option_b.id for key in movers) - assert all(new_assignment[key] == option_a.id for key in movers) + assert all(original_assignment[key] == option_b.key for key in movers) + assert all(new_assignment[key] == option_a.key for key in movers) assert all( new_assignment[key] == original_assignment[key] - for key in identity_hash_keys + for key in original_assignment if key not in movers ) diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py index d4fb731421e4..d79edeb84312 100644 --- a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py @@ -23,6 +23,7 @@ from audit.related_object_type import RelatedObjectType from cohorts.models import Cohort from core.helpers import get_current_site_url +from environments.identities.models import Identity from environments.models import Environment from features.models import Feature, FeatureSegment, FeatureState from features.versioning.models import ( @@ -48,6 +49,7 @@ # TODO: Delete alias as per https://github.com/Flagsmith/flagsmith/issues/7818 from segments.types import SegmentRule as SegmentRuleType +from tests.types import VariantAssignmentFixture from users.models import FFAdminUser now = timezone.now() @@ -1242,20 +1244,26 @@ def test_change_request_commit__v1_multivariate_feature__keeps_variant_bucketing environment: Environment, multivariate_feature: Feature, admin_user: FFAdminUser, + variant_assignment: VariantAssignmentFixture, ) -> None: # Given the current live environment-default feature state of a multivariate # feature, and the variant each of a range of identities is bucketed into + # The fixture derives an option's value from its percentage, so two of them + # share a value. Key them so a variant identifies which option won. + for index, option in enumerate(multivariate_feature.multivariate_options.all()): + option.key = f"variant-{index}" + option.save() live_feature_state = FeatureState.objects.get( environment=environment, feature=multivariate_feature, identity=None, feature_segment=None, ) - identity_hash_keys = [f"identity-{i}" for i in range(50)] - original_assignment = { - key: live_feature_state.get_multivariate_feature_state_value(key).pk - for key in identity_hash_keys - } + identities = [ + Identity.objects.create(identifier=f"identity-{i}", environment=environment) + for i in range(50) + ] + original_assignment = variant_assignment(identities, multivariate_feature.name) # and a change request carrying a draft feature state for the same feature, # as created by the API when a change request is raised under v1 versioning @@ -1285,11 +1293,9 @@ def test_change_request_commit__v1_multivariate_feature__keeps_variant_bucketing assert new_live_feature_state.mv_hashing_salt == live_feature_state.id # and every identity stays in the same variant as before the commit - new_assignment = { - key: new_live_feature_state.get_multivariate_feature_state_value(key).pk - for key in identity_hash_keys - } - assert new_assignment == original_assignment + assert variant_assignment(identities, multivariate_feature.name) == ( + original_assignment + ) def test_change_request_commit__v1_segment_override_draft__inherits_mv_hashing_salt( diff --git a/api/tests/unit/import_export/test_unit_import_export_export.py b/api/tests/unit/import_export/test_unit_import_export_export.py index 157833be68f0..5e595b3c1931 100644 --- a/api/tests/unit/import_export/test_unit_import_export_export.py +++ b/api/tests/unit/import_export/test_unit_import_export_export.py @@ -17,6 +17,7 @@ from core.constants import STRING from environments.identities.models import Identity from environments.models import Environment, EnvironmentAPIKey, Webhook +from evaluation.services import get_identity_feature_states from features.feature_types import MULTIVARIATE from features.models import Feature, FeatureSegment, FeatureState from features.multivariate.models import MultivariateFeatureOption @@ -539,37 +540,31 @@ def test_export_edge_identities__identities_with_overrides_and_traits__exports_a assert bool_trait.trait_key == "bool_trait" assert bool_trait.trait_value is True - all_feature_states = identity.get_all_feature_states() + all_feature_states = get_identity_feature_states(identity) assert len(all_feature_states) == 7 actual_mv_override = all_feature_states[0] assert str(actual_mv_override.uuid) == mv_override_fs_uuid - assert ( - actual_mv_override.get_feature_state_value(identity=identity) - == mv_option.string_value - ) + assert actual_mv_override.evaluated_value == mv_option.string_value actual_int_override = all_feature_states[1] assert str(actual_int_override.uuid) == int_override_fs_uuid - assert actual_int_override.get_feature_state_value(identity=identity) == 123 + assert actual_int_override.evaluated_value == 123 actual_float_override = all_feature_states[2] assert str(actual_float_override.uuid) == float_override_fs_uuid - assert actual_float_override.get_feature_state_value(identity=identity) == "123.123" + assert actual_float_override.evaluated_value == "123.123" actual_bool_override = all_feature_states[3] assert str(actual_bool_override.uuid) == bool_override_fs_uuid - assert actual_bool_override.get_feature_state_value(identity=identity) is False + assert actual_bool_override.evaluated_value is False actual_string_fs = all_feature_states[4] - assert actual_string_fs.get_feature_state_value(identity=identity) == "foo" + assert actual_string_fs.evaluated_value == "foo" assert actual_string_fs.identity is None override_without_mv_option = all_feature_states[5] - assert ( - override_without_mv_option.get_feature_state_value(identity=identity) - == "control" - ) + assert override_without_mv_option.evaluated_value == "control" assert override_without_mv_option.identity == identity override_with_missing_attributes = all_feature_states[6] diff --git a/api/tests/unit/integrations/heap/test_unit_heap.py b/api/tests/unit/integrations/heap/test_unit_heap.py index ae450f47ca5f..f3b341bca40b 100644 --- a/api/tests/unit/integrations/heap/test_unit_heap.py +++ b/api/tests/unit/integrations/heap/test_unit_heap.py @@ -4,6 +4,7 @@ from environments.identities.models import Identity from environments.models import Environment +from evaluation.services import get_identity_feature_states from features.models import FeatureState from integrations.heap.heap import HeapWrapper from integrations.heap.models import HeapConfiguration @@ -91,7 +92,7 @@ def test_identify_integrations__heap_configured__posts_to_expected_url( mocked_post = mocker.patch("integrations.heap.heap.requests.post") # When - identify_integrations(identity, identity.get_all_feature_states()) # type: ignore[no-untyped-call] + identify_integrations(identity, get_identity_feature_states(identity)) # type: ignore[no-untyped-call] # Then assert mocked_post.call_args.args[0] == expected_url diff --git a/api/tests/unit/integrations/mixpanel/test_unit_mixpanel.py b/api/tests/unit/integrations/mixpanel/test_unit_mixpanel.py index aca925dc0c85..72cf3d2874d0 100644 --- a/api/tests/unit/integrations/mixpanel/test_unit_mixpanel.py +++ b/api/tests/unit/integrations/mixpanel/test_unit_mixpanel.py @@ -3,6 +3,7 @@ import pytest +from evaluation.services import get_identity_feature_states from integrations.mixpanel.constants import DEFAULT_MIXPANEL_API_URL from integrations.mixpanel.mixpanel import MixpanelWrapper from integrations.mixpanel.models import MixpanelConfiguration @@ -194,7 +195,7 @@ def test_identify_integrations__mixpanel_configured__posts_to_expected_url( mocked_post = mocker.patch("integrations.mixpanel.mixpanel.requests.post") # When - identify_integrations(identity, identity.get_all_feature_states()) # type: ignore[no-untyped-call] + identify_integrations(identity, get_identity_feature_states(identity)) # type: ignore[no-untyped-call] # Then assert mocked_post.call_args.args[0] == expected_url diff --git a/api/tests/unit/integrations/test_unit_integration.py b/api/tests/unit/integrations/test_unit_integration.py index 3029d30e6f8f..50c93028a478 100644 --- a/api/tests/unit/integrations/test_unit_integration.py +++ b/api/tests/unit/integrations/test_unit_integration.py @@ -1,3 +1,4 @@ +from evaluation.services import get_identity_feature_states from integrations.amplitude.models import AmplitudeConfiguration from integrations.common.models import EnvironmentIntegrationModel from integrations.common.wrapper import AbstractBaseIdentityIntegrationWrapper @@ -15,7 +16,7 @@ def test_identify_integrations__amplitude_configured__calls_amplitude( # type: AmplitudeConfiguration.objects.create(api_key="abc-123", environment=environment) # When - identify_integrations(identity, identity.get_all_feature_states()) # type: ignore[no-untyped-call] + identify_integrations(identity, get_identity_feature_states(identity)) # type: ignore[no-untyped-call] # Then mock_amplitude_wrapper.assert_called() @@ -30,7 +31,7 @@ def test_identify_integrations__segment_configured__calls_segment( # type: igno ) SegmentConfiguration.objects.create(api_key="abc-123", environment=environment) # When - identify_integrations(identity, identity.get_all_feature_states()) # type: ignore[no-untyped-call] + identify_integrations(identity, get_identity_feature_states(identity)) # type: ignore[no-untyped-call] # Then mock_segment_wrapper.assert_called() @@ -66,7 +67,7 @@ def test_identify_integrations__multiple_integrations__calls_all( # type: ignor ) # When - identify_integrations(identity, identity.get_all_feature_states()) # type: ignore[no-untyped-call] + identify_integrations(identity, get_identity_feature_states(identity)) # type: ignore[no-untyped-call] # Then # Integration a was successfully called @@ -79,7 +80,7 @@ def test_identify_integrations__multiple_integrations__calls_all( # type: ignor integration_a_mocked_generate_user_data.assert_called_with( identity=identity, - feature_states=identity.get_all_feature_states(), + feature_states=get_identity_feature_states(identity), trait_models=None, ) integration_wrapper_a.return_value.identify_user_async.assert_called_with( @@ -95,7 +96,7 @@ def test_identify_integrations__multiple_integrations__calls_all( # type: ignor integration_b_mocked_generate_user_data.assert_called_with( identity=identity, - feature_states=identity.get_all_feature_states(), + feature_states=get_identity_feature_states(identity), trait_models=None, ) integration_wrapper_b.return_value.identify_user_async.assert_called_with( @@ -115,7 +116,7 @@ def test_identify_integrations__deleted_integration__does_not_call( # type: ign sc.delete() # When - identify_integrations(identity, identity.get_all_feature_states()) # type: ignore[no-untyped-call] + identify_integrations(identity, get_identity_feature_states(identity)) # type: ignore[no-untyped-call] # Then mock_segment_wrapper.assert_not_called() diff --git a/api/tests/unit/integrations/webhook/test_unit_webhook_serializers.py b/api/tests/unit/integrations/webhook/test_unit_webhook_serializers.py index 99fbe3a226a2..401bc5997b5b 100644 --- a/api/tests/unit/integrations/webhook/test_unit_webhook_serializers.py +++ b/api/tests/unit/integrations/webhook/test_unit_webhook_serializers.py @@ -6,14 +6,19 @@ def test_integration_feature_state_serializer__multivariate_feature__returns_correct_weight( # type: ignore[no-untyped-def] - identity, multivariate_feature, mocker + identity, multivariate_feature ): # Given mv_option = multivariate_feature.multivariate_options.first() - mocker.patch.object( - FeatureState, "get_multivariate_feature_state_value", return_value=mv_option - ) feature_state = FeatureState.objects.filter(feature=multivariate_feature).first() + # The identity was evaluated into the first option. + feature_state.flag_result = { + "name": multivariate_feature.name, + "enabled": feature_state.enabled, + "value": mv_option.value, + "reason": "DEFAULT", + "variant": mv_option.key, + } # When serializer = IntegrationFeatureStateSerializer( diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index 8dc792563315..4df1d283b3b9 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -3,6 +3,7 @@ from collections.abc import Callable from copy import deepcopy from datetime import timedelta +from unittest.mock import MagicMock import freezegun import pytest @@ -13,6 +14,7 @@ from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ObjectDoesNotExist from django.urls import reverse from django.utils import timezone from flag_engine.segments.constants import EQUAL @@ -518,8 +520,8 @@ def test_list_segments__filter_by_edge_identity__returns_only_matching_segments( identity_uuid = identity_document["identity_uuid"] assert isinstance(identity_uuid, str) - edge_identity_dynamo_wrapper_mock.get_segment_ids.return_value = ( - expected_segment_ids + edge_identity_dynamo_wrapper_mock.get_item_from_uuid.return_value = ( + identity_document ) base_url = reverse("api-v1:projects:project-segments-list", args=[project.id]) @@ -531,7 +533,32 @@ def test_list_segments__filter_by_edge_identity__returns_only_matching_segments( # Then assert response.json().get("count") == len(expected_segment_ids) assert response.json()["results"][0]["id"] == expected_segment_ids[0] - edge_identity_dynamo_wrapper_mock.get_segment_ids.assert_called_with(identity_uuid) + edge_identity_dynamo_wrapper_mock.get_item_from_uuid.assert_called_with( + identity_uuid + ) + + +def test_list_segments__filter_by_unknown_edge_identity__returns_no_segments( + project: Project, + environment: Environment, + identity_matching_segment: Segment, + edge_identity_dynamo_wrapper_mock: MagicMock, + admin_client: APIClient, +) -> None: + # Given + edge_identity_dynamo_wrapper_mock.get_item_from_uuid.side_effect = ( + ObjectDoesNotExist + ) + base_url = reverse("api-v1:projects:project-segments-list", args=[project.id]) + + # When + response = admin_client.get( + f"{base_url}?identity=8ce1e2f8-0a0f-4f5c-9f4c-2b4f6f4f4f4f" + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["count"] == 0 @pytest.mark.parametrize( diff --git a/api/tests/unit/util/engine_models/context/test_unit_context_mappers.py b/api/tests/unit/util/engine_models/context/test_unit_context_mappers.py deleted file mode 100644 index db2b2d57bbd4..000000000000 --- a/api/tests/unit/util/engine_models/context/test_unit_context_mappers.py +++ /dev/null @@ -1,32 +0,0 @@ -from environments.models import Environment -from util.engine_models.context.mappers import map_environment_identity_to_context -from util.engine_models.identities.models import IdentityModel -from util.engine_models.identities.traits.models import TraitModel - - -def test_map_environment_identity_to_context__system_traits__merged_with_system_winning( - environment: Environment, -) -> None: - # Given - identity = IdentityModel( - identifier="user-1", - environment_api_key=environment.api_key, - identity_traits=[ - TraitModel(trait_key="plan", trait_value="free"), - TraitModel(trait_key="flagsmith_cohort_a", trait_value="user-written"), - ], - system_traits={"flagsmith_cohort_a": True}, - ) - - # When - context = map_environment_identity_to_context( - environment=environment, identity=identity, override_traits=None - ) - - # Then - identity_context = context["identity"] - assert identity_context is not None - assert identity_context["traits"] == { - "plan": "free", - "flagsmith_cohort_a": True, - } diff --git a/api/tests/unit/util/mappers/test_unit_mappers_engine.py b/api/tests/unit/util/mappers/test_unit_mappers_engine.py index ef5bf4e62f79..5f1f0a96d0ed 100644 --- a/api/tests/unit/util/mappers/test_unit_mappers_engine.py +++ b/api/tests/unit/util/mappers/test_unit_mappers_engine.py @@ -18,7 +18,7 @@ from integrations.mixpanel.models import MixpanelConfiguration from integrations.segment.models import SegmentConfiguration from integrations.webhook.models import WebhookConfiguration -from segments.models import Condition, Segment, SegmentRule +from segments.models import Segment, SegmentRule from users.models import FFAdminUser from util.engine_models.environments.integrations.models import IntegrationModel from util.engine_models.environments.models import ( @@ -801,210 +801,6 @@ def test_map_environment_to_engine__v2_versioning_segment_override_removed__retu ) -def test_map_environment_to_evaluation_context__no_identity__returns_environment_only( - environment: Environment, -) -> None: - # Given / When - result = engine.map_environment_to_evaluation_context(environment=environment) - - # Then - assert result == { - "environment": { - "key": environment.api_key, - "name": environment.name, - }, - } - - -def test_map_environment_to_evaluation_context__with_identity__returns_identity_context( - environment: Environment, - identity: Identity, -) -> None: - # Given / When - result = engine.map_environment_to_evaluation_context( - environment=environment, - identity=identity, - ) - - # Then - assert result == { - "environment": { - "key": environment.api_key, - "name": environment.name, - }, - "identity": { - "identifier": identity.identifier, - "key": identity.get_hash_key( - environment.use_identity_composite_key_for_hashing - ), - "traits": {}, - }, - } - - -def test_map_environment_to_evaluation_context__with_explicit_traits__returns_given_traits( - environment: Environment, - identity: Identity, - trait: Trait, -) -> None: - # Given / When - result = engine.map_environment_to_evaluation_context( - environment=environment, - identity=identity, - traits=[trait], - ) - - # Then - assert result == { - "environment": { - "key": environment.api_key, - "name": environment.name, - }, - "identity": { - "identifier": identity.identifier, - "key": identity.get_hash_key( - environment.use_identity_composite_key_for_hashing - ), - "traits": {trait.trait_key: trait.trait_value}, - }, - } - - -def test_map_environment_to_evaluation_context__no_explicit_traits__returns_identity_traits( - environment: Environment, - identity: Identity, - trait: Trait, -) -> None: - # Given / When - result = engine.map_environment_to_evaluation_context( - environment=environment, - identity=identity, - ) - - # Then - assert result == { - "environment": { - "key": environment.api_key, - "name": environment.name, - }, - "identity": { - "identifier": identity.identifier, - "key": identity.get_hash_key( - environment.use_identity_composite_key_for_hashing - ), - "traits": {trait.trait_key: trait.trait_value}, - }, - } - - -def test_map_environment_to_evaluation_context__with_segments__returns_segment_contexts( - environment: Environment, - identity_matching_segment: Segment, -) -> None: - # Given / When - result = engine.map_environment_to_evaluation_context( - environment=environment, - segments=[identity_matching_segment], - ) - - # Then - segment_key = str(identity_matching_segment.pk) - assert result == { - "environment": { - "key": environment.api_key, - "name": environment.name, - }, - "segments": { - segment_key: engine.map_segment_to_segment_context( - identity_matching_segment - ), - }, - } - - -def test_map_segment_to_segment_context__segment_with_rule__returns_expected( - identity_matching_segment: Segment, -) -> None: - # Given - condition = Condition.objects.get( - rule__segment=identity_matching_segment, - ) - - # When - result = engine.map_segment_to_segment_context(identity_matching_segment) - - # Then - assert result == { - "key": str(identity_matching_segment.pk), - "name": identity_matching_segment.name, - "rules": [ - { - "type": "ALL", - "conditions": [ - { - "property": condition.property, - "operator": condition.operator, - "value": condition.value, - }, - ], - "rules": [], - }, - ], - "metadata": {"pk": identity_matching_segment.pk}, - } - - -def test_map_rule_to_segment_rule__with_nested_rule__returns_expected( - segment_rule: SegmentRule, - identity_matching_segment: Segment, -) -> None: - # Given - matching_rule = SegmentRule.objects.get(segment=identity_matching_segment) - matching_rule.rules.add(segment_rule) - condition = Condition.objects.get(rule=matching_rule) - - # When - result = engine.map_rule_to_segment_rule(matching_rule) - - # Then - assert result == { - "type": "ALL", - "conditions": [ - { - "property": condition.property, - "operator": condition.operator, - "value": condition.value, - }, - ], - "rules": [ - { - "type": "ALL", - "conditions": [], - "rules": [], - }, - ], - } - - -def test_map_condition_to_segment_condition__valid_condition__returns_expected( - identity_matching_segment: Segment, -) -> None: - # Given - condition = Condition.objects.get( - rule__segment=identity_matching_segment, - ) - - # When - result = engine.map_condition_to_segment_condition(condition) - - # Then - assert result == { - "property": condition.property, - "operator": condition.operator, - "value": condition.value, - } - - def test_map_environment_to_engine__running_experiment__stamps_every_state_of_feature( environment: Environment, feature: "Feature", diff --git a/api/util/engine_models/context/mappers.py b/api/util/engine_models/context/mappers.py index 0f6301981bfd..86a243d5774e 100644 --- a/api/util/engine_models/context/mappers.py +++ b/api/util/engine_models/context/mappers.py @@ -8,7 +8,6 @@ import typing from flag_engine.context.types import ( - EvaluationContext, FeatureContext, SegmentContext, SegmentRule, @@ -18,53 +17,8 @@ FeatureStateModel, MultivariateFeatureStateValueModel, ) -from util.engine_models.identities.models import IdentityModel -from util.engine_models.identities.traits.models import TraitModel from util.engine_models.segments.models import SegmentModel, SegmentRuleModel -if typing.TYPE_CHECKING: - from environments.models import Environment - - -def map_environment_identity_to_context( - environment: "Environment", - identity: IdentityModel, - override_traits: typing.Optional[typing.List[TraitModel]], -) -> EvaluationContext: - """ - Map an environment and IdentityModel to an EvaluationContext. - - Vendored from flagsmith-flag-engine's fix/missing-export branch and adapted - to return v10's EvaluationContext TypedDict. - - :param environment: An Environment object. - :param identity: The identity model object (Pydantic IdentityModel). - :param override_traits: A list of TraitModel objects, to be used in place of - `identity.identity_traits` if provided. - :return: An EvaluationContext containing the environment and identity. - """ - traits = { - trait.trait_key: trait.trait_value - for trait in ( - override_traits if override_traits is not None else identity.identity_traits - ) - } - if identity.system_traits: - # System-owned traits are not user data: on a key clash, the system - # value wins. - traits.update(identity.system_traits) - return { - "environment": { - "key": environment.api_key, - "name": environment.name or "", - }, - "identity": { - "identifier": identity.identifier, - "key": str(identity.django_id or identity.composite_key), - "traits": traits, - }, - } - def _map_feature_states_to_feature_contexts( feature_states: typing.List[FeatureStateModel], @@ -160,26 +114,3 @@ def map_segment_to_segment_context(segment: SegmentModel) -> SegmentContext: _map_feature_states_to_feature_contexts(segment_feature_states).values() ) return segment_ctx - - -# TODO: Migrate to get_evaluation_result - see #6669 -def is_context_in_segment( - context: EvaluationContext, - segment: SegmentModel, -) -> bool: - """ - Check if an evaluation context matches a segment. - - This is a compatibility wrapper that bridges the Pydantic SegmentModel - with the v10 flag-engine's TypedDict-based evaluation API. - - :param context: The EvaluationContext (TypedDict). - :param segment: The SegmentModel (Pydantic model). - :return: True if the context matches the segment rules. - """ - from flag_engine.segments.evaluator import ( - is_context_in_segment as v10_is_context_in_segment, - ) - - segment_context = map_segment_to_segment_context(segment) - return v10_is_context_in_segment(context, segment_context) diff --git a/api/util/mappers/engine.py b/api/util/mappers/engine.py index 55afadd9f186..77ffc7a6bb02 100644 --- a/api/util/mappers/engine.py +++ b/api/util/mappers/engine.py @@ -3,13 +3,8 @@ from typing import TYPE_CHECKING, Dict, List, Optional from uuid import UUID -from flag_engine.context import types as engine_types -from flag_engine.segments.types import ConditionOperator, RuleType -from pydantic import TypeAdapter - from environments.constants import IDENTITY_INTEGRATIONS_RELATION_NAMES from features.versioning.models import EnvironmentFeatureVersion -from segments.types import SegmentEngineMetadata from util.engine_models.environments.integrations.models import IntegrationModel from util.engine_models.environments.models import ( EnvironmentAPIKeyModel, @@ -48,20 +43,16 @@ from integrations.webhook.models import WebhookConfiguration from organisations.models import Organisation from projects.models import Project - from segments.models import Condition, Segment, SegmentRule + from segments.models import Segment, SegmentRule __all__ = ( - "map_condition_to_segment_condition", "map_environment_api_key_to_engine", "map_environment_to_engine", "map_feature_to_engine", "map_identity_to_engine", - "map_environment_to_evaluation_context", "map_mv_option_to_engine", - "map_rule_to_segment_rule", "map_segment_to_engine", - "map_segment_to_segment_context", "map_traits_to_engine", ) @@ -450,83 +441,6 @@ def map_identity_to_engine( ) -_rule_type_adapter: TypeAdapter[RuleType] = TypeAdapter(RuleType) -_condition_operator_adapter: TypeAdapter[ConditionOperator] = TypeAdapter( - ConditionOperator -) - - -def map_environment_to_evaluation_context( - *, - environment: "Environment", - identity: "Identity | None" = None, - traits: "Iterable[Trait] | None" = None, - segments: "Iterable[Segment] | None" = None, -) -> "engine_types.EvaluationContext[SegmentEngineMetadata, object]": - """Map Django ORM Environment (and optionally Identity) to a flag-engine EvaluationContext.""" - context: engine_types.EvaluationContext[SegmentEngineMetadata, object] = { - "environment": { - "key": environment.api_key, - "name": environment.name or "", - }, - } - if identity is not None: - trait_items: "Iterable[Trait]" = ( - traits if traits is not None else identity.identity_traits.all() - ) - identity_traits = {trait.trait_key: trait.trait_value for trait in trait_items} - if identity.system_traits: - # System-owned traits are not user data: on a key clash, the system - # value wins. - identity_traits.update(identity.system_traits) - context["identity"] = { - "identifier": identity.identifier, - "key": identity.get_hash_key( - environment.use_identity_composite_key_for_hashing - ), - "traits": identity_traits, - } - if segments is not None: - context["segments"] = { - str(segment.pk): map_segment_to_segment_context(segment) - for segment in segments - } - return context - - -def map_segment_to_segment_context( - segment: "Segment", -) -> "engine_types.SegmentContext[SegmentEngineMetadata, object]": - """Map a Django ORM Segment to a flag-engine SegmentContext TypedDict.""" - return { - "key": str(segment.pk), - "name": segment.name, - "rules": [map_rule_to_segment_rule(rule) for rule in segment.rules.all()], - "metadata": SegmentEngineMetadata(pk=segment.pk), - } - - -def map_rule_to_segment_rule(rule: "SegmentRule") -> engine_types.SegmentRule: - return { - "type": _rule_type_adapter.validate_python(rule.type), - "conditions": [ - map_condition_to_segment_condition(condition) - for condition in rule.conditions.all() - ], - "rules": [map_rule_to_segment_rule(sub_rule) for sub_rule in rule.rules.all()], - } - - -def map_condition_to_segment_condition( - condition: "Condition", -) -> engine_types.StrValueSegmentCondition: - return { - "property": condition.property or "", - "operator": _condition_operator_adapter.validate_python(condition.operator), - "value": condition.value or "", - } - - def _get_prioritised_feature_states( feature_states: Iterable["FeatureState"], ) -> List["FeatureState"]: diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 79394941051d..665d8364c1e2 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -676,7 +676,7 @@ Attributes: ### `segments.delete_rejected` Logged at `warning` from: - - `api/segments/views.py:279` + - `api/segments/views.py:292` Attributes: - `organisation.id` @@ -696,7 +696,7 @@ Attributes: ### `segments.update_rejected` Logged at `warning` from: - - `api/segments/views.py:255` + - `api/segments/views.py:268` Attributes: - `organisation.id`