Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions products/feature_flags/backend/test/test_flags_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@
_REPO_ROOT = Path(__file__).resolve().parents[4]


def _extract_schema(obj: Any) -> Any:

Check warning on line 1181 in products/feature_flags/backend/test/test_flags_cache.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

lint:complexity

`_extract_schema` has cyclomatic complexity 14 (warn >10)

Check warning on line 1181 in products/feature_flags/backend/test/test_flags_cache.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

`_extract_schema` has cyclomatic complexity 14 (warn >10)
"""Extract a type skeleton from a JSON-like object for structural comparison.

Returns a nested structure of dicts, lists, and leaf type names.
Expand Down Expand Up @@ -1225,7 +1225,7 @@
return type(obj).__name__


def _compare_schemas(fixture_schema: Any, result_schema: Any, path: str = "") -> list[str]:

Check warning on line 1228 in products/feature_flags/backend/test/test_flags_cache.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

lint:complexity

`_compare_schemas` has cyclomatic complexity 12 (warn >10)

Check warning on line 1228 in products/feature_flags/backend/test/test_flags_cache.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

`_compare_schemas` has cyclomatic complexity 12 (warn >10)
"""Compare two type skeletons and return human-readable diffs."""
diffs: list[str] = []
# If the fixture declares a field as null, the serializer must also return null.
Expand Down Expand Up @@ -1317,7 +1317,7 @@
super().setUp()
clear_flags_cache(self.team, kinds=["redis", "s3"])

def test_serializer_output_matches_fixture_schema(self):

Check warning on line 1320 in products/feature_flags/backend/test/test_flags_cache.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

lint:complexity

`test_serializer_output_matches_fixture_schema` has cyclomatic complexity 12 (warn >10)

Check warning on line 1320 in products/feature_flags/backend/test/test_flags_cache.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

`test_serializer_output_matches_fixture_schema` has cyclomatic complexity 12 (warn >10)
"""Golden fixture validation: Python serializer output must match the fixture schema.

Recursively compares the type skeleton (keys + value types at every nesting
Expand Down Expand Up @@ -2437,14 +2437,19 @@
created_by=self.user,
filters={"groups": [{"properties": [], "rollout_percentage": 100}]},
)
ctx = EvaluationContext.objects.create(team=self.team, name="original-context-name")
# Out of alphabetical order on purpose. The assertions below also cover the sort that
# ArrayAgg(distinct=True) applies, which the Rust builder matches with
# ARRAY_AGG(DISTINCT ...). Remove distinct=True and the two write different arrays.
ctx = EvaluationContext.objects.create(team=self.team, name="zulu-context")
other_ctx = EvaluationContext.objects.create(team=self.team, name="alpha-context")
FeatureFlagEvaluationContext.objects.create(feature_flag=flag, evaluation_context=ctx)
FeatureFlagEvaluationContext.objects.create(feature_flag=flag, evaluation_context=other_ctx)

# Warm the cache
update_flags_cache(self.team)

# Rename the context directly in DB (bypassing signals to simulate stale cache)
EvaluationContext.objects.filter(id=ctx.id).update(name="renamed-context-name")
EvaluationContext.objects.filter(id=ctx.id).update(name="mike-context")

# Verify should detect the mismatch
result = verify_team_flags(self.team, verbose=True)
Expand All @@ -2456,8 +2461,8 @@

field_diffs = result["diffs"][0]["field_diffs"]
eval_tag_diff = next(d for d in field_diffs if d["field"] == "evaluation_contexts")
self.assertEqual(eval_tag_diff["cached_value"], ["original-context-name"])
self.assertEqual(eval_tag_diff["db_value"], ["renamed-context-name"])
self.assertEqual(eval_tag_diff["cached_value"], ["alpha-context", "zulu-context"])
self.assertEqual(eval_tag_diff["db_value"], ["alpha-context", "mike-context"])

# Mismatch result should include db_data for cache fix optimization
self.assertIn("db_data", result)
Expand Down
17 changes: 11 additions & 6 deletions rust/feature-flags/src/flags/feature_flag_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,15 @@ impl FeatureFlagList {
f.ensure_experience_continuity,
f.version,
f.evaluation_runtime,
-- DISTINCT makes Postgres sort the names before it aggregates them, which
-- matches ArrayAgg(..., distinct=True) on the two Python paths that write
-- this cache entry: products/feature_flags/backend/flags_cache.py (batch) and
-- products/feature_flags/backend/models/feature_flag.py (single team, which is
-- also what the cache verifier compares against). Without DISTINCT the array
-- order follows the join, so the sides disagree for a flag with two or more
-- contexts. Keep all three in lockstep.
COALESCE(
ARRAY_AGG(ctx.name) FILTER (WHERE ctx.name IS NOT NULL),
ARRAY_AGG(DISTINCT ctx.name) FILTER (WHERE ctx.name IS NOT NULL),
'{}'::text[]
) AS evaluation_tags,
bucketing_identifier,
Expand Down Expand Up @@ -504,15 +511,13 @@ mod tests {
assert_eq!(flag.key, "flag1");
assert_eq!(flag.team_id, team.id);

// Check that evaluation tags were properly fetched
// ARRAY_AGG(DISTINCT ...) sorts, so this is not the insertion order.
let tags = flag
.evaluation_tags
.as_ref()
.expect("Should have evaluation tags");
assert_eq!(tags.len(), 3);
assert!(tags.contains(&"docs-page".to_string()));
assert!(tags.contains(&"marketing-site".to_string()));
assert!(tags.contains(&"app".to_string()));
let tag_names: Vec<&str> = tags.iter().map(String::as_str).collect();
assert_eq!(tag_names, ["app", "docs-page", "marketing-site"]);
}

#[tokio::test]
Expand Down
Loading