Skip to content

Commit f1af8bb

Browse files
committed
feat: Support $.flags context values in segment conditions
Environment flags are now evaluated with segments whose conditions all read `$.environment` or `$.flags`, as `GET /api/v1/flags/` does since Flagsmith/flagsmith#8576. Segments reading traits or identity context are left out. When segment conditions read `$.flags`, identity flags are evaluated at once against the full context, as a context trimmed to one feature lacks the flags it depends on. Otherwise, they are still evaluated lazily. Closes #245
1 parent e80d1c4 commit f1af8bb

4 files changed

Lines changed: 171 additions & 30 deletions

File tree

‎flagsmith/flagsmith.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,12 @@ def _get_identity_flags_from_document(
513513
identifier=identifier,
514514
traits=traits,
515515
)
516+
if local_evaluation.reads.flags:
517+
return Flags.from_evaluation_result(
518+
evaluation_result=engine.get_evaluation_result(context=context),
519+
analytics_processor=self._analytics_processor,
520+
default_flag_handler=self.default_flag_handler,
521+
)
516522
# Lazy: defer per-feature evaluation until the caller actually reads
517523
# a flag. Hot for callers that only read one or a few flags out of a
518524
# large environment.

‎flagsmith/local_evaluation.py‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from functools import cached_property
66

77
from flag_engine import engine
8-
from flag_engine.context.types import SegmentContext
8+
from flag_engine.context.types import SegmentContext, SegmentRule
99

1010
from flagsmith.api.types import EnvironmentModel
1111
from flagsmith.mappers import (
@@ -15,6 +15,7 @@
1515
)
1616
from flagsmith.models import (
1717
Flag,
18+
SegmentContexts,
1819
SegmentOverridesIndex,
1920
build_segment_overrides_index,
2021
)
@@ -25,6 +26,35 @@
2526
TraitMapping,
2627
)
2728

29+
_FLAGS_PROPERTY_PREFIXES = ("$.flags.", "$.flags[")
30+
_IDENTITY_FREE_PROPERTY_PREFIXES = (
31+
"$.environment.",
32+
"$.environment[",
33+
*_FLAGS_PROPERTY_PREFIXES,
34+
)
35+
36+
37+
class _ContextReads(typing.NamedTuple):
38+
"""The context values segment rules read."""
39+
40+
flags: bool
41+
identity: bool
42+
43+
44+
def _get_context_reads(rules: typing.Iterable[SegmentRule]) -> _ContextReads:
45+
reads_flags = reads_identity = False
46+
for rule in rules:
47+
for condition in rule.get("conditions") or ():
48+
condition_property = condition.get("property") or ""
49+
reads_flags |= condition_property.startswith(_FLAGS_PROPERTY_PREFIXES)
50+
reads_identity |= not condition_property.startswith(
51+
_IDENTITY_FREE_PROPERTY_PREFIXES
52+
)
53+
nested_reads = _get_context_reads(rule.get("rules") or ())
54+
reads_flags |= nested_reads.flags
55+
reads_identity |= nested_reads.identity
56+
return _ContextReads(flags=reads_flags, identity=reads_identity)
57+
2858

2959
class LocalEvaluation:
3060
"""Per-environment-document state for local evaluation."""
@@ -37,6 +67,15 @@ def __init__(
3767
] = None,
3868
) -> None:
3969
self.context = context
70+
self._identity_free_segments: SegmentContexts = {}
71+
reads_flags = reads_identity = False
72+
for segment_key, segment_context in (context.get("segments") or {}).items():
73+
reads = _get_context_reads(segment_context["rules"])
74+
reads_flags |= reads.flags
75+
reads_identity |= reads.identity
76+
if not reads.identity:
77+
self._identity_free_segments[segment_key] = segment_context
78+
self.reads = _ContextReads(flags=reads_flags, identity=reads_identity)
4079
self.overrides_index: SegmentOverridesIndex = build_segment_overrides_index(
4180
context
4281
)
@@ -64,10 +103,8 @@ def from_environment_document(
64103
@cached_property
65104
def environment_flags(self) -> typing.Dict[str, Flag]:
66105
"""Environment flags, which depend on the environment document only."""
67-
context_without_segments = self.context.copy()
68-
context_without_segments.pop("segments", None)
69106
evaluation_result = engine.get_evaluation_result(
70-
context=context_without_segments,
107+
context={**self.context, "segments": self._identity_free_segments},
71108
)
72109
return {
73110
feature_name: Flag.from_evaluation_result(flag_result)

‎tests/test_flagsmith.py‎

Lines changed: 93 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
FlagsmithFeatureDoesNotExistError,
1919
)
2020
from flagsmith.local_evaluation import LocalEvaluation
21+
from flagsmith.mappers import map_environment_document_to_context
2122
from flagsmith.models import DefaultFlag, ExperimentMetadata, Flag, Flags
2223
from flagsmith.offline_handlers import OfflineHandler
2324
from flagsmith.types import SDKEvaluationContext
@@ -101,40 +102,72 @@ def test_get_environment_flags_uses_local_environment_when_available(
101102
assert all_flags[0].value == "some-value"
102103

103104

104-
def test_get_environment_flags_omits_segments_from_evaluation_context(
105+
def _condition(property_: str) -> typing.Any:
106+
return {"operator": "EQUAL", "property_": property_, "value": "value"}
107+
108+
109+
@pytest.mark.parametrize(
110+
"rules, expected_segment_names",
111+
[
112+
(
113+
[{"type": "ALL", "conditions": [_condition("$.environment.name")]}],
114+
["Test segment"],
115+
),
116+
(
117+
[
118+
{
119+
"type": "ALL",
120+
"rules": [
121+
{
122+
"type": "ANY",
123+
"conditions": [
124+
_condition("$.environment['name']"),
125+
_condition("$.flags.some_feature.enabled"),
126+
],
127+
}
128+
],
129+
}
130+
],
131+
["Test segment"],
132+
),
133+
([{"type": "ALL", "conditions": [_condition("trait")]}], []),
134+
(
135+
[
136+
{
137+
"type": "ALL",
138+
"conditions": [_condition("$.environment.name")],
139+
"rules": [{"type": "ALL", "conditions": [_condition("trait")]}],
140+
}
141+
],
142+
[],
143+
),
144+
],
145+
)
146+
def test_get_environment_flags__local_evaluation__includes_identity_free_segments_only(
105147
mocker: MockerFixture,
106-
local_eval_flagsmith: Flagsmith,
107-
evaluation_context: SDKEvaluationContext,
148+
flagsmith: Flagsmith,
149+
environment: EnvironmentModel,
150+
rules: typing.Any,
151+
expected_segment_names: typing.List[str],
108152
) -> None:
109153
# Given
110-
mock_get_evaluation_result = mocker.patch(
111-
"flagsmith.flagsmith.engine.get_evaluation_result",
112-
autospec=True,
154+
# the only segment has the given rules, plus identity overrides
155+
environment["project"]["segments"][0]["rules"] = rules
156+
flagsmith._local_evaluation = LocalEvaluation(
157+
map_environment_document_to_context(environment)
113158
)
114-
115-
expected_evaluation_result = {
116-
"flags": {
117-
"some_feature": {
118-
"name": "some_feature",
119-
"enabled": True,
120-
"value": "some-feature-state-value",
121-
"reason": "DEFAULT",
122-
"metadata": {"id": 1},
123-
}
124-
},
125-
"segments": [],
126-
}
127-
128-
mock_get_evaluation_result.return_value = expected_evaluation_result
159+
flagsmith.enable_local_evaluation = True
160+
spy = mocker.spy(engine, "get_evaluation_result")
129161

130162
# When
131-
local_eval_flagsmith.get_environment_flags()
163+
flagsmith.get_environment_flags()
132164

133165
# Then
134-
# Verify segments are not present in the context passed to the engine
135-
context_without_segments = evaluation_context.copy()
136-
context_without_segments.pop("segments", None)
137-
mock_get_evaluation_result.assert_called_once_with(context=context_without_segments)
166+
# only segments reading `$.environment` or `$.flags` alone are evaluated,
167+
# as `GET /api/v1/flags/` does
168+
context = spy.call_args.kwargs["context"]
169+
segments = (context.get("segments") or {}).values()
170+
assert [segment["name"] for segment in segments] == expected_segment_names
138171

139172

140173
def test_get_environment_flags__local_evaluation__evaluates_once_per_environment(
@@ -316,6 +349,40 @@ def test_get_identity_flags_includes_segments_in_evaluation_context(
316349
assert "segments" in context
317350

318351

352+
def test_get_identity_flags__flags_condition__evaluates_full_context(
353+
mocker: MockerFixture,
354+
flagsmith: Flagsmith,
355+
environment: EnvironmentModel,
356+
) -> None:
357+
# Given
358+
# a segment condition reading another flag
359+
environment["project"]["segments"][0]["rules"] = [
360+
{
361+
"type": "ALL",
362+
"rules": [],
363+
"conditions": [_condition("$.flags.some_feature.enabled")],
364+
}
365+
]
366+
flagsmith._local_evaluation = LocalEvaluation(
367+
map_environment_document_to_context(environment)
368+
)
369+
flagsmith.enable_local_evaluation = True
370+
spy = mocker.spy(engine, "get_evaluation_result")
371+
372+
# When
373+
flagsmith.get_identity_flags("identifier").get_flag("some_feature")
374+
375+
# Then
376+
# flags are evaluated at once against the full context, segments included,
377+
# rather than per flag against a trimmed one
378+
spy.assert_called_once()
379+
context = spy.call_args.kwargs["context"]
380+
assert set(context["features"]) == {"some_feature"}
381+
assert [segment["name"] for segment in (context["segments"] or {}).values()] == [
382+
"Test segment"
383+
]
384+
385+
319386
def test_get_identity_flags__resolves_one_flag_at_a_time(
320387
local_eval_flagsmith: Flagsmith,
321388
mocker: MockerFixture,

‎tests/test_local_evaluation.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import copy
2+
13
import pytest
24
from flag_engine import engine
35
from pytest_mock import MockerFixture
@@ -141,3 +143,32 @@ def test_local_evaluation__environment_flags__evaluated_once(
141143
assert first is second
142144
assert first["some_feature"].value == "some-value"
143145
spy.assert_called_once()
146+
147+
148+
@pytest.mark.parametrize(
149+
"condition_property, expected_reads_flags",
150+
[
151+
("$.flags.some_feature.enabled", True),
152+
("$.flags['some feature'].value", True),
153+
("$.flagship", False),
154+
("flags", False),
155+
],
156+
)
157+
def test_local_evaluation__reads__flags__returns_expected(
158+
environment: EnvironmentModel,
159+
condition_property: str,
160+
expected_reads_flags: bool,
161+
) -> None:
162+
# Given
163+
# a condition in a nested rule of the only segment
164+
environment = copy.deepcopy(environment)
165+
condition = environment["project"]["segments"][0]["rules"][0]["rules"][0][
166+
"conditions"
167+
][0]
168+
condition["property_"] = condition_property
169+
170+
# When
171+
local_evaluation = LocalEvaluation.from_environment_document(environment)
172+
173+
# Then
174+
assert local_evaluation.reads.flags is expected_reads_flags

0 commit comments

Comments
 (0)