diff --git a/flagsmith/flagsmith.py b/flagsmith/flagsmith.py index d8f34b2..1c4717d 100644 --- a/flagsmith/flagsmith.py +++ b/flagsmith/flagsmith.py @@ -15,6 +15,7 @@ EventProcessorConfig, ) from flagsmith.exceptions import FlagsmithAPIError, FlagsmithClientError +from flagsmith.local_evaluation import LocalEvaluation from flagsmith.mappers import ( map_context_and_identity_data_to_context, map_environment_document_to_context, @@ -22,14 +23,7 @@ map_segment_results_to_identity_segments, resolve_trait_values, ) -from flagsmith.models import ( - DefaultFlag, - Flag, - Flags, - Segment, - SegmentOverridesIndex, - build_segment_overrides_index, -) +from flagsmith.models import DefaultFlag, Flag, Flags, Segment from flagsmith.offline_handlers import OfflineHandler from flagsmith.polling_manager import EnvironmentDataPollingManager from flagsmith.streaming_manager import EventStreamManager @@ -136,7 +130,7 @@ def __init__( self._analytics_processor: typing.Optional[AnalyticsProcessor] = None self._event_processor: typing.Optional[EventProcessor] = None self.__evaluation_context: typing.Optional[SDKEvaluationContext] = None - self._segment_overrides_index: SegmentOverridesIndex = {} + self._local_evaluation: typing.Optional[LocalEvaluation] = None self._environment_updated_at: typing.Optional[datetime] = None # argument validation @@ -341,13 +335,15 @@ def get_identity_segments( Flagsmith, e.g. {"num_orders": 10} :return: list of Segment objects that the identity is part of. """ - if not self._evaluation_context: + if not (local_evaluation := self._local_evaluation): raise FlagsmithClientError( "Local evaluation required to obtain identity segments." ) + # Identity overrides segments are left out: they aren't segments + # in Flagsmith, so they're never returned. context = map_context_and_identity_data_to_context( - context=self._evaluation_context, + context=local_evaluation.base_context, identifier=identifier, traits=traits, ) @@ -489,17 +485,16 @@ def _evaluation_context(self) -> typing.Optional[SDKEvaluationContext]: def _evaluation_context( self, context: typing.Optional[SDKEvaluationContext] ) -> None: - """Swap in a new evaluation context and rebuild the overrides index. + """Swap in a new evaluation context and rebuild local evaluation state. - The index maps feature_name -> segments that override it. Built once - per refresh and reused across every subsequent per-identity lazy - resolution; rebuilding here keeps it in sync with the current doc - without any hot-path cost. + `LocalEvaluation` holds everything derived from the environment + document, e.g. the segment overrides index. Rebuilt once per refresh + and reused across every evaluation, without any hot-path cost. """ - self.__evaluation_context = context - self._segment_overrides_index = ( - build_segment_overrides_index(context) if context is not None else {} + self._local_evaluation = ( + LocalEvaluation(context) if context is not None else None ) + self.__evaluation_context = context def _get_headers( self, @@ -520,22 +515,15 @@ def _get_headers( return headers def _get_environment_flags_from_document(self) -> Flags: - if self._evaluation_context is None: + if (local_evaluation := self._local_evaluation) is None: raise TypeError("No environment present") - # Omit segments from evaluation context for environment flags - # as they are only relevant for identity-specific evaluations - context_without_segments = self._evaluation_context.copy() - context_without_segments.pop("segments", None) - - evaluation_result = engine.get_evaluation_result( - context=context_without_segments, - ) - - return Flags.from_evaluation_result( - evaluation_result=evaluation_result, - analytics_processor=self._analytics_processor, + # Evaluated once per environment document. `Flag` instances are + # shared across calls; the mapping holding them is not. + return Flags( + flags=dict(local_evaluation.environment_flags), default_flag_handler=self.default_flag_handler, + _analytics_processor=self._analytics_processor, ) def _get_identity_flags_from_document( @@ -543,11 +531,10 @@ def _get_identity_flags_from_document( identifier: str, traits: TraitMapping, ) -> Flags: - if self._evaluation_context is None: + if (local_evaluation := self._local_evaluation) is None: raise TypeError("No environment present") - context = map_context_and_identity_data_to_context( - context=self._evaluation_context, + context, overrides_index = local_evaluation.get_identity_context( identifier=identifier, traits=traits, ) @@ -556,7 +543,7 @@ def _get_identity_flags_from_document( # large environment. return Flags.from_evaluation_context( context=context, - overrides_index=self._segment_overrides_index, + overrides_index=overrides_index, analytics_processor=self._analytics_processor, default_flag_handler=self.default_flag_handler, ) diff --git a/flagsmith/local_evaluation.py b/flagsmith/local_evaluation.py new file mode 100644 index 0000000..c91eec4 --- /dev/null +++ b/flagsmith/local_evaluation.py @@ -0,0 +1,114 @@ +from __future__ import annotations + +import typing +from collections import ChainMap +from functools import cached_property + +from flag_engine import engine +from flag_engine.context.types import SegmentContext + +from flagsmith.mappers import ( + get_identity_overrides_identifiers, + map_context_and_identity_data_to_context, + map_identifiers_to_identity_overrides_rules, +) +from flagsmith.models import ( + Flag, + SegmentContexts, + SegmentOverridesIndex, + build_segment_overrides_index, +) +from flagsmith.types import ( + FeatureMetadata, + SDKEvaluationContext, + SegmentMetadata, + TraitMapping, +) + + +class LocalEvaluation: + """Per-environment-document state for local evaluation. + + Built once each time the environment document is refreshed, so the + work here is amortised across every evaluation until the next refresh. + + Identity overrides are mapped to segments matching the identifiers + they apply to. Left in the context, every one of them would be + matched on every identity evaluation. Instead, they're indexed by + identifier, and each identity is evaluated with only its own. + """ + + def __init__(self, context: SDKEvaluationContext) -> None: + self.context = context + + segments: SegmentContexts = {} + self._identity_overrides_segment_keys: typing.Dict[str, typing.List[str]] = {} + for segment_key, segment_context in (context.get("segments") or {}).items(): + identifiers = get_identity_overrides_identifiers(segment_context) + if identifiers is None: + segments[segment_key] = segment_context + continue + for identifier in identifiers: + self._identity_overrides_segment_keys.setdefault(identifier, []).append( + segment_key + ) + + #: The context without identity overrides segments. + self.base_context: SDKEvaluationContext = {**context, "segments": segments} + self.overrides_index: SegmentOverridesIndex = build_segment_overrides_index( + self.base_context + ) + + @cached_property + def environment_flags(self) -> typing.Dict[str, Flag]: + """Environment flags, which depend on the environment document only.""" + # Omit segments from evaluation context for environment flags + # as they are only relevant for identity-specific evaluations + context_without_segments = self.context.copy() + context_without_segments.pop("segments", None) + evaluation_result = engine.get_evaluation_result( + context=context_without_segments, + ) + return { + feature_name: Flag.from_evaluation_result(flag_result) + for feature_name, flag_result in evaluation_result["flags"].items() + } + + def get_identity_context( + self, + identifier: str, + traits: typing.Optional[TraitMapping] = None, + ) -> typing.Tuple[SDKEvaluationContext, SegmentOverridesIndex]: + """Evaluation context and overrides index for an identity. + + The context carries the identity's own identity overrides segments, + narrowed to its identifier, so that their size doesn't depend on how + many identities share the same overrides. + """ + context = map_context_and_identity_data_to_context( + context=self.base_context, + identifier=identifier, + traits=traits, + ) + if not (segment_keys := self._identity_overrides_segment_keys.get(identifier)): + return context, self.overrides_index + + identity_segments: SegmentContexts = {} + identity_overrides_index: SegmentOverridesIndex = {} + segment_contexts = self.context.get("segments") or {} + for segment_key in segment_keys: + segment_context: SegmentContext[SegmentMetadata, FeatureMetadata] = { + **segment_contexts[segment_key], + "rules": map_identifiers_to_identity_overrides_rules([identifier]), + } + identity_segments[segment_key] = segment_context + for override in segment_context.get("overrides") or (): + feature_name = override["name"] + if feature_name not in identity_overrides_index: + identity_overrides_index[feature_name] = dict( + self.overrides_index.get(feature_name) or {} + ) + identity_overrides_index[feature_name][segment_key] = segment_context + + context["segments"] = {**(context.get("segments") or {}), **identity_segments} + return context, ChainMap(identity_overrides_index, self.overrides_index) diff --git a/flagsmith/mappers.py b/flagsmith/mappers.py index 091cf3e..73d8454 100644 --- a/flagsmith/mappers.py +++ b/flagsmith/mappers.py @@ -179,18 +179,7 @@ def _map_identity_overrides_to_segments( segment_contexts[segment_key] = SegmentContext( key=segment_key, name="identity_overrides", - rules=[ - { - "type": "ALL", - "conditions": [ - { - "property": "$.identity.identifier", - "operator": "IN", - "value": identifiers, - } - ], - } - ], + rules=map_identifiers_to_identity_overrides_rules(identifiers), overrides=[ { "key": "", # Identity overrides never carry multivariate options @@ -207,6 +196,36 @@ def _map_identity_overrides_to_segments( return segment_contexts +def map_identifiers_to_identity_overrides_rules( + identifiers: typing.List[str], +) -> typing.List[SegmentRule]: + return [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.identity.identifier", + "operator": "IN", + "value": identifiers, + } + ], + } + ] + + +def get_identity_overrides_identifiers( + segment_context: SegmentContext[SegmentMetadata, FeatureMetadata], +) -> typing.Optional[typing.List[str]]: + """Identifiers an identity-overrides segment applies to, or `None` for + any other segment.""" + if (segment_context.get("metadata") or {}).get("source") != "identity_overrides": + return None + # As built by `map_identifiers_to_identity_overrides_rules`. + [rule] = segment_context["rules"] + [condition] = rule["conditions"] + return typing.cast(typing.List[str], condition["value"]) + + def _map_environment_document_rules_to_context_rules( rules: list[SegmentRuleModel], ) -> list[SegmentRule]: diff --git a/flagsmith/models.py b/flagsmith/models.py index d29f86c..b1b5ded 100644 --- a/flagsmith/models.py +++ b/flagsmith/models.py @@ -16,9 +16,10 @@ SegmentMetadata, ) -SegmentOverridesIndex = typing.Dict[ - str, typing.Dict[str, SegmentContext[SegmentMetadata, FeatureMetadata]] -] +#: Segment contexts by their key in the evaluation context. +SegmentContexts = typing.Dict[str, SegmentContext[SegmentMetadata, FeatureMetadata]] +#: Feature name -> segment contexts overriding that feature. +SegmentOverridesIndex = typing.MutableMapping[str, SegmentContexts] def build_segment_overrides_index( diff --git a/tests/test_flagsmith.py b/tests/test_flagsmith.py index 9296feb..266d457 100644 --- a/tests/test_flagsmith.py +++ b/tests/test_flagsmith.py @@ -134,6 +134,33 @@ def test_get_environment_flags_omits_segments_from_evaluation_context( mock_get_evaluation_result.assert_called_once_with(context=context_without_segments) +def test_get_environment_flags__local_evaluation__evaluates_once_per_environment( + mocker: MockerFixture, + local_eval_flagsmith: Flagsmith, + evaluation_context: SDKEvaluationContext, +) -> None: + # Given + spy = mocker.spy(engine, "get_evaluation_result") + + # When + first = local_eval_flagsmith.get_environment_flags() + second = local_eval_flagsmith.get_environment_flags() + + # Then + # the environment is evaluated once, each call getting its own mapping + assert spy.call_count == 1 + assert first.flags == second.flags + assert first.flags is not second.flags + + # When + # the environment is refreshed + local_eval_flagsmith._evaluation_context = evaluation_context + local_eval_flagsmith.get_environment_flags() + + # Then + assert spy.call_count == 2 + + @responses.activate() def test_get_identity_flags_calls_api_when_no_local_environment_no_traits( flagsmith: Flagsmith, identities_json: str diff --git a/tests/test_local_evaluation.py b/tests/test_local_evaluation.py new file mode 100644 index 0000000..e67feaa --- /dev/null +++ b/tests/test_local_evaluation.py @@ -0,0 +1,127 @@ +import copy + +import pytest +from flag_engine import engine +from pytest_mock import MockerFixture + +from flagsmith.api.types import EnvironmentModel +from flagsmith.local_evaluation import LocalEvaluation +from flagsmith.mappers import map_environment_document_to_context + + +@pytest.fixture() +def local_evaluation(environment: EnvironmentModel) -> LocalEvaluation: + """From data/environment.json, plus an identity sharing the overrides of + `overridden-id`, so both map to the same identity overrides segment.""" + environment = copy.deepcopy(environment) + sharing_identity = copy.deepcopy(environment["identity_overrides"][0]) + sharing_identity["identifier"] = "sharing-overridden-id" + environment["identity_overrides"].append(sharing_identity) + return LocalEvaluation(map_environment_document_to_context(environment)) + + +def test_local_evaluation__base_context__excludes_identity_overrides_segments( + local_evaluation: LocalEvaluation, +) -> None: + # When + base_segments = local_evaluation.base_context["segments"] + + # Then + assert base_segments is not None + assert [segment["name"] for segment in base_segments.values()] == ["Test segment"] + assert local_evaluation.overrides_index == {} + + +def test_local_evaluation__get_identity_context__no_identity_overrides__returns_base( + local_evaluation: LocalEvaluation, +) -> None: + # When + context, overrides_index = local_evaluation.get_identity_context( + "not-overridden-id", {"foo": "bar"} + ) + + # Then + assert context["segments"] is local_evaluation.base_context["segments"] + assert overrides_index is local_evaluation.overrides_index + assert context["identity"] == { + "identifier": "not-overridden-id", + "traits": {"foo": "bar"}, + } + + +@pytest.mark.parametrize( + "identifier", ["overridden-id", "sharing-overridden-id", "another-overridden-id"] +) +def test_local_evaluation__get_identity_context__identity_overrides__returns_own_narrowed( + local_evaluation: LocalEvaluation, + identifier: str, +) -> None: + # When + context, overrides_index = local_evaluation.get_identity_context(identifier) + + # Then + # only the identity's own identity overrides segment is added, + # matching its identifier alone + assert context["segments"] is not None + identity_segments = [ + segment + for segment in context["segments"].values() + if segment["name"] == "identity_overrides" + ] + assert len(identity_segments) == 1 + [segment] = identity_segments + assert segment["rules"][0]["conditions"] == [ + { + "property": "$.identity.identifier", + "operator": "IN", + "value": [identifier], + } + ] + assert list(overrides_index) == ["some_feature"] + assert list(overrides_index["some_feature"].values()) == [segment] + + # And the shared state is left untouched + assert local_evaluation.overrides_index == {} + assert local_evaluation.base_context["segments"] is not None + assert len(local_evaluation.base_context["segments"]) == 1 + + +@pytest.mark.parametrize( + "identifier, expected_value", + [ + ("overridden-id", "some-overridden-value"), + ("sharing-overridden-id", "some-overridden-value"), + ("another-overridden-id", "another-overridden-value"), + ("not-overridden-id", "some-value"), + ], +) +def test_local_evaluation__get_identity_context__evaluates_to_identity_overrides( + local_evaluation: LocalEvaluation, + identifier: str, + expected_value: str, +) -> None: + # Given + context, _ = local_evaluation.get_identity_context(identifier) + + # When + result = engine.get_evaluation_result(context) + + # Then + assert result["flags"]["some_feature"]["value"] == expected_value + + +def test_local_evaluation__environment_flags__evaluated_once( + local_evaluation: LocalEvaluation, + mocker: MockerFixture, +) -> None: + # Given + spy = mocker.spy(engine, "get_evaluation_result") + + # When + first = local_evaluation.environment_flags + second = local_evaluation.environment_flags + + # Then + assert first is second + assert first["some_feature"].value == "some-value" + spy.assert_called_once()