From a819d0c5c9c02f37ec08b2524fec8eaf76d8e3a3 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Mon, 14 Sep 2026 09:27:04 -0300 Subject: [PATCH 01/21] maybe the chicken came first --- api/app/settings/common.py | 1 + api/features/dependencies/__init__.py | 0 api/features/dependencies/apps.py | 7 + .../dependencies/migrations/0001_initial.py | 48 ++ .../dependencies/migrations/__init__.py | 0 api/features/dependencies/models.py | 26 + .../dependencies/test_segment_management.py | 632 ++++++++++++++++++ 7 files changed, 714 insertions(+) create mode 100644 api/features/dependencies/__init__.py create mode 100644 api/features/dependencies/apps.py create mode 100644 api/features/dependencies/migrations/0001_initial.py create mode 100644 api/features/dependencies/migrations/__init__.py create mode 100644 api/features/dependencies/models.py create mode 100644 api/tests/integration/features/dependencies/test_segment_management.py diff --git a/api/app/settings/common.py b/api/app/settings/common.py index 0ebbc0ec3cc1..e6eb43ef262c 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -115,6 +115,7 @@ "environments.identities", "environments.identities.traits", "features", + "features.dependencies", "features.feature_external_resources", "features.feature_health", "features.import_export", diff --git a/api/features/dependencies/__init__.py b/api/features/dependencies/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/api/features/dependencies/apps.py b/api/features/dependencies/apps.py new file mode 100644 index 000000000000..6f0e0ffb3bbe --- /dev/null +++ b/api/features/dependencies/apps.py @@ -0,0 +1,7 @@ +from core.apps import BaseAppConfig + + +class FeatureDependenciesConfig(BaseAppConfig): + name = "features.dependencies" + label = "feature_dependencies" + default = True diff --git a/api/features/dependencies/migrations/0001_initial.py b/api/features/dependencies/migrations/0001_initial.py new file mode 100644 index 000000000000..f6df7c453c95 --- /dev/null +++ b/api/features/dependencies/migrations/0001_initial.py @@ -0,0 +1,48 @@ +# Generated by Django 5.2.17 on 2026-09-14 18:40 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ("features", "0067_add_feature_state_mv_hashing_salt"), + ("segments", "0032_add_segment_rules_data"), + ] + + operations = [ + migrations.CreateModel( + name="SegmentFlagReference", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("condition_json_path", models.TextField()), + ( + "prerequisite_feature", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="segment_references", + to="features.feature", + ), + ), + ( + "segment", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="flag_references", + to="segments.segment", + ), + ), + ], + ), + ] diff --git a/api/features/dependencies/migrations/__init__.py b/api/features/dependencies/migrations/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/api/features/dependencies/models.py b/api/features/dependencies/models.py new file mode 100644 index 000000000000..cec48e38b925 --- /dev/null +++ b/api/features/dependencies/models.py @@ -0,0 +1,26 @@ +from django.db import models + + +class SegmentFlagReference(models.Model): + """A reference to a pre-requisite flag from the segment + + When a segment rule points to a feature as a pre-requisite, an object of + this type must exist to materialise the relationship and enable easier + backreferencing, e.g. for validating circular dependencies, and querying + without inferring from JSON rules. + """ + + segment = models.ForeignKey( + "segments.Segment", + on_delete=models.CASCADE, + related_name="flag_references", + ) + + prerequisite_feature = models.ForeignKey( + "features.Feature", + on_delete=models.CASCADE, + related_name="segment_references", + ) + + # JSONPath (RFC 9535) locating the rule condition + condition_json_path = models.TextField() diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py new file mode 100644 index 000000000000..30dcc5f44a69 --- /dev/null +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -0,0 +1,632 @@ +import pytest +from pytest_structlog import StructuredLogCapture +from rest_framework.test import APIClient + +from features.dependencies.models import SegmentFlagReference +from features.models import Feature, FeatureSegment +from projects.models import Project +from segments.models import Segment + + +@pytest.fixture( + params=[False, True], + ids=["change_requests_disabled", "change_requests_enabled"], +) +def with_and_without_segment_change_requests( + request: pytest.FixtureRequest, + project: int, +) -> bool: + enabled: bool = request.param + if enabled: + Project.objects.filter(id=project).update(minimum_change_request_approvals=0) + return enabled + + +@pytest.mark.usefixtures("with_and_without_segment_change_requests") +def test_create_segment__valid_flag_dependency__indexes_created( + admin_client: APIClient, + project: int, +) -> None: + # Given + flag_a = Feature.objects.create(name="flag_a", project_id=project) + flag_b = Feature.objects.create(name="flag_b", project_id=project) + flag_c = Feature.objects.create(name="flag_c", project_id=project) + + # When + response = admin_client.post( + f"/api/v1/projects/{project}/segments/", + data={ + "name": "power_users", + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.flag_a.enabled", + "operator": "EQUAL", + "value": True, + }, + ], + "rules": [ + { + "type": "ANY", + "conditions": [ + { + "property": "$.flags.flag_b.enabled", + "operator": "EQUAL", + "value": True, + }, + { + "property": "$.flags['flag_c'].enabled", + "operator": "EQUAL", + "value": True, + }, + ], + }, + ], + } + ], + }, + format="json", + ) + + # Then + assert response.status_code == 201 + segment_id = response.json()["id"] + assert list( + SegmentFlagReference.objects.values( + "segment", + "prerequisite_feature", + "condition_json_path", + ) + ) == [ + { + "segment": segment_id, + "prerequisite_feature": flag_a.id, + "condition_json_path": "$[0].conditions[0]", + }, + { + "segment": segment_id, + "prerequisite_feature": flag_b.id, + "condition_json_path": "$[0].rules[0].conditions[0]", + }, + { + "segment": segment_id, + "prerequisite_feature": flag_c.id, + "condition_json_path": "$[0].rules[0].conditions[1]", + }, + ] + + +@pytest.mark.usefixtures("with_and_without_segment_change_requests") +def test_create_segment__nonexistent_prerequisite__responds_400( + admin_client: APIClient, + project: int, +) -> None: + # Given / When + response = admin_client.post( + f"/api/v1/projects/{project}/segments/", + data={ + "name": "segment", + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.unicorn.enabled", + "operator": "EQUAL", + "value": True, + }, + ], + } + ], + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "code": "prerequisite_feature_not_found", + "prerequisite_feature": "unicorn", + "condition_json_path": "$[0].conditions[0]", + } + assert not Segment.objects.exists() + assert not SegmentFlagReference.objects.exists() + + +def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( + admin_client: APIClient, + environment: int, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + rooster = Feature.objects.create(name="rooster", project_id=project) + segment = Segment.objects.create(name="segment", project_id=project) + SegmentFlagReference.objects.create( + segment=segment, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[0]", + ) + FeatureSegment.objects.create( + segment=segment, + feature=chicken, + environment_id=environment, + ) + + # When + response = admin_client.put( + f"/api/v1/projects/{project}/segments/{segment.id}/", + data={ + "name": "segment", + "project": project, + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.rooster.enabled", + "operator": "EQUAL", + "value": True, + }, + ], + } + ], + }, + format="json", + ) + + # Then + assert response.status_code == 200 + assert list( + SegmentFlagReference.objects.values( + "segment", + "prerequisite_feature", + "condition_json_path", + ) + ) == [ + { + "segment": segment.id, + "prerequisite_feature": rooster.id, + "condition_json_path": "$[0].conditions[0]", + }, + ] + assert log.has( + "dependencies.deleted", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="chicken", + prerequisite_feature__name="egg", + ) + assert log.has( + "dependencies.created", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="chicken", + prerequisite_feature__name="rooster", + ) + + +def test_delete_segment__flag_dependency__indexes_removed( + admin_client: APIClient, + environment: int, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + segment = Segment.objects.create(name="segment", project_id=project) + SegmentFlagReference.objects.create( + segment=segment, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[0]", + ) + FeatureSegment.objects.create( + segment=segment, + feature=chicken, + environment_id=environment, + ) + + # When + response = admin_client.delete( + f"/api/v1/projects/{project}/segments/{segment.id}/", + ) + + # Then + assert response.status_code == 204 + assert not SegmentFlagReference.objects.exists() + assert log.has( + "dependencies.deleted", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="chicken", + prerequisite_feature__name="egg", + ) + + +@pytest.mark.usefixtures("with_and_without_segment_change_requests") +def test_update_segment_add_override__circular_flag_dependency__responds_400( + admin_client: APIClient, + environment: int, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + segment1 = Segment.objects.create(name="segment1", project_id=project) + segment2 = Segment.objects.create(name="segment2", project_id=project) + SegmentFlagReference.objects.create( + segment=segment1, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[1]", + ) + FeatureSegment.objects.create( + segment=segment1, + feature=chicken, + environment_id=environment, + ) + SegmentFlagReference.objects.create( + segment=segment2, + prerequisite_feature=chicken, + condition_json_path="$[1].conditions[2]", + ) + + # When + response = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{egg.id}/create-segment-override/", + data={ + "feature_state_value": {}, + "feature_segment": {"segment": segment2.id}, + "enabled": True, + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "code": "circular_dependency", + "path": [ + { + "feature": "egg", + "needs": "chicken", + "segment": { + "id": segment2.id, + "name": segment2.name, + "condition_json_path": "$[1].conditions[2]", + }, + }, + { + "feature": "chicken", + "needs": "egg", + "segment": { + "id": segment1.id, + "name": segment1.name, + "condition_json_path": "$[0].conditions[1]", + }, + }, + ], + } + assert not FeatureSegment.objects.filter(segment=segment2, feature=egg).exists() + assert log.has( + "dependencies.create_failed", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="egg", + prerequisite_feature__name="chicken", + ) + + +@pytest.mark.usefixtures("with_and_without_segment_change_requests") +def test_update_segment_add_override__longer_dependency_cycle_path__responds_400( + admin_client: APIClient, + environment: int, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + rooster = Feature.objects.create(name="rooster", project_id=project) + segment1 = Segment.objects.create(name="segment1", project_id=project) + segment2 = Segment.objects.create(name="segment2", project_id=project) + segment3 = Segment.objects.create(name="segment3", project_id=project) + SegmentFlagReference.objects.create( + segment=segment1, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[0]", + ) + FeatureSegment.objects.create( + segment=segment1, + feature=chicken, + environment_id=environment, + ) + SegmentFlagReference.objects.create( + segment=segment2, + prerequisite_feature=rooster, + condition_json_path="$[0].conditions[0]", + ) + FeatureSegment.objects.create( + segment=segment2, + feature=egg, + environment_id=environment, + ) + SegmentFlagReference.objects.create( + segment=segment3, + prerequisite_feature=chicken, + condition_json_path="$[0].conditions[0]", + ) + + # When + response = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{rooster.id}/create-segment-override/", + data={ + "feature_state_value": {}, + "feature_segment": {"segment": segment3.id}, + "enabled": True, + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "code": "circular_dependency", + "path": [ + { + "feature": "rooster", + "needs": "chicken", + "segment": { + "id": segment3.id, + "name": segment3.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + { + "feature": "chicken", + "needs": "egg", + "segment": { + "id": segment1.id, + "name": segment1.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + { + "feature": "egg", + "needs": "rooster", + "segment": { + "id": segment2.id, + "name": segment2.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + ], + } + assert not FeatureSegment.objects.filter(segment=segment3, feature=rooster).exists() + assert log.has( + "dependencies.create_failed", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="rooster", + prerequisite_feature__name="chicken", + ) + + +def test_update_segment_update_rules__circular_flag_dependency__responds_400( + admin_client: APIClient, + environment: int, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + segment1 = Segment.objects.create(name="segment1", project_id=project) + segment2 = Segment.objects.create(name="segment2", project_id=project) + SegmentFlagReference.objects.create( + segment=segment1, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[1]", + ) + FeatureSegment.objects.create( + segment=segment1, + feature=chicken, + environment_id=environment, + ) + FeatureSegment.objects.create( + segment=segment2, + feature=egg, + environment_id=environment, + ) + + # When + response = admin_client.put( + f"/api/v1/projects/{project}/segments/{segment2.id}/", + data={ + "name": "segment2", + "project": project, + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.chicken.enabled", + "operator": "EQUAL", + "value": True, + }, + ], + } + ], + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "code": "circular_dependency", + "path": [ + { + "feature": "egg", + "needs": "chicken", + "segment": { + "id": segment2.id, + "name": segment2.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + { + "feature": "chicken", + "needs": "egg", + "segment": { + "id": segment1.id, + "name": segment1.name, + "condition_json_path": "$[0].conditions[1]", + }, + }, + ], + } + assert not SegmentFlagReference.objects.filter(segment=segment2).exists() + assert log.has( + "dependencies.create_failed", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="egg", + prerequisite_feature__name="chicken", + ) + + +def test_update_segment_update_rules__longer_dependency_cycle_path__responds_400( + admin_client: APIClient, + environment: int, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + rooster = Feature.objects.create(name="rooster", project_id=project) + segment1 = Segment.objects.create(name="segment1", project_id=project) + segment2 = Segment.objects.create(name="segment2", project_id=project) + segment3 = Segment.objects.create(name="segment3", project_id=project) + SegmentFlagReference.objects.create( + segment=segment1, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[0]", + ) + FeatureSegment.objects.create( + segment=segment1, + feature=chicken, + environment_id=environment, + ) + SegmentFlagReference.objects.create( + segment=segment2, + prerequisite_feature=rooster, + condition_json_path="$[0].conditions[0]", + ) + FeatureSegment.objects.create( + segment=segment2, + feature=egg, + environment_id=environment, + ) + FeatureSegment.objects.create( + segment=segment3, + feature=rooster, + environment_id=environment, + ) + + # When + response = admin_client.put( + f"/api/v1/projects/{project}/segments/{segment3.id}/", + data={ + "name": "segment3", + "project": project, + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.chicken.enabled", + "operator": "EQUAL", + "value": True, + }, + ], + } + ], + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "code": "circular_dependency", + "path": [ + { + "feature": "rooster", + "needs": "chicken", + "segment": { + "id": segment3.id, + "name": segment3.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + { + "feature": "chicken", + "needs": "egg", + "segment": { + "id": segment1.id, + "name": segment1.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + { + "feature": "egg", + "needs": "rooster", + "segment": { + "id": segment2.id, + "name": segment2.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + ], + } + assert not SegmentFlagReference.objects.filter(segment=segment3).exists() + assert log.has( + "dependencies.create_failed", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="rooster", + prerequisite_feature__name="chicken", + ) From ec3f473b5be1348b5e1a463e2e021932b35de86d Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Tue, 15 Sep 2026 18:04:20 -0300 Subject: [PATCH 02/21] index prerequisites --- api/features/dependencies/services.py | 74 +++++++++++++++++++++++++++ api/segments/serializers.py | 2 + 2 files changed, 76 insertions(+) create mode 100644 api/features/dependencies/services.py diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py new file mode 100644 index 000000000000..cc3145e70c2c --- /dev/null +++ b/api/features/dependencies/services.py @@ -0,0 +1,74 @@ +import jsonpath_rfc9535 +from jsonpath_rfc9535.exceptions import JSONPathError +from jsonpath_rfc9535.segments import JSONPathChildSegment, JSONPathSegment +from jsonpath_rfc9535.selectors import NameSelector + +from features.dependencies.models import SegmentFlagReference +from features.models import Feature +from segments.models import Segment +from segments.types import SegmentRule + +JSONPathStr = str + + +def index_segment_flag_references(segment: Segment) -> None: + """Materialise the segment's `$.flags` conditions as SegmentFlagReference rows.""" + feature_names_by_json_path = { + f"{rule_json_path}.conditions[{condition_index}]": feature_name + for rule_json_path, rule in _get_rules_by_json_path( + segment.rules_data or [] + ).items() + for condition_index, condition in enumerate(rule["conditions"]) + if (condition_property := condition["property"]) + and (feature_name := _get_prerequisite_feature_name(condition_property)) + } + feature_ids_by_name = dict( + Feature.objects.filter( + project_id=segment.project_id, + name__in=set(feature_names_by_json_path.values()), + ).values_list("name", "id") + ) + SegmentFlagReference.objects.bulk_create( + SegmentFlagReference( + segment=segment, + prerequisite_feature_id=feature_ids_by_name[feature_name], + condition_json_path=condition_json_path, + ) + for condition_json_path, feature_name in feature_names_by_json_path.items() + ) + + +def _get_rules_by_json_path( + rules: list[SegmentRule], +) -> dict[JSONPathStr, SegmentRule]: + rules_by_json_path: dict[JSONPathStr, SegmentRule] = {} + for rule_index, rule in enumerate(rules): + rule_json_path = f"$[{rule_index}]" + rules_by_json_path[rule_json_path] = rule + for nested_index, nested_rule in enumerate(rule.get("rules", [])): + rules_by_json_path[f"{rule_json_path}.rules[{nested_index}]"] = nested_rule + return rules_by_json_path + + +def _get_prerequisite_feature_name(condition_property: str) -> str | None: + """Return the feature name a `$.flags.` condition points at, if it does.""" + # Because of historical decisions, `$['flags']` can be a trait. + if not condition_property.startswith("$.flags"): + return None + try: + query_segments = jsonpath_rfc9535.compile(condition_property).segments + except JSONPathError: + return None + if len(query_segments) < 2 or _get_selected_name(query_segments[0]) != "flags": + return None + return _get_selected_name(query_segments[1]) + + +def _get_selected_name(query_segment: JSONPathSegment) -> str | None: + if ( + not isinstance(query_segment, JSONPathChildSegment) + or len(query_segment.selectors) != 1 + ): + return None + selector = query_segment.selectors[0] + return selector.name if isinstance(selector, NameSelector) else None diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 4364967a3b3e..c1984e1414f8 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -10,6 +10,7 @@ from cohorts.models import Cohort from edge_api.utils import is_edge_enabled +from features.dependencies.services import index_segment_flag_references from metadata.serializers import MetadataSerializer, MetadataSerializerMixin from projects.models import Project from segment_membership.constants import MAX_SEGMENT_MEMBERS_PAGE_SIZE @@ -218,6 +219,7 @@ def create(self, validated_data: dict[str, Any]): # type: ignore[no-untyped-def metadata_data = validated_data.pop("metadata", []) self._set_rules_data(validated_data) segment = super().create(validated_data) # type: ignore[no-untyped-call] + index_segment_flag_references(segment) self._update_metadata(segment, metadata_data) enqueue_membership_refresh(segment.project) return segment From d61176b9d5cf6a56f6b22f4e082506e42585c771 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Tue, 15 Sep 2026 18:23:27 -0300 Subject: [PATCH 03/21] you can't depend on the unknown --- api/features/dependencies/exceptions.py | 18 ++++++++++++++++++ api/features/dependencies/services.py | 7 +++++++ api/segments/serializers.py | 5 +++-- 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 api/features/dependencies/exceptions.py diff --git a/api/features/dependencies/exceptions.py b/api/features/dependencies/exceptions.py new file mode 100644 index 000000000000..a3647056312b --- /dev/null +++ b/api/features/dependencies/exceptions.py @@ -0,0 +1,18 @@ +from rest_framework import status +from rest_framework.exceptions import APIException + + +class PrerequisiteFeatureNotFoundError(APIException): + """Raised where a segment condition names a feature that does not exist.""" + + status_code = status.HTTP_400_BAD_REQUEST + default_code = "prerequisite_feature_not_found" + + def __init__(self, prerequisite_feature: str, condition_json_path: str) -> None: + super().__init__( + { + "code": self.default_code, + "prerequisite_feature": prerequisite_feature, + "condition_json_path": condition_json_path, + } + ) diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index cc3145e70c2c..07b1dadb2e22 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -3,6 +3,7 @@ from jsonpath_rfc9535.segments import JSONPathChildSegment, JSONPathSegment from jsonpath_rfc9535.selectors import NameSelector +from features.dependencies.exceptions import PrerequisiteFeatureNotFoundError from features.dependencies.models import SegmentFlagReference from features.models import Feature from segments.models import Segment @@ -28,6 +29,12 @@ def index_segment_flag_references(segment: Segment) -> None: name__in=set(feature_names_by_json_path.values()), ).values_list("name", "id") ) + for condition_json_path, feature_name in feature_names_by_json_path.items(): + if feature_name not in feature_ids_by_name: + raise PrerequisiteFeatureNotFoundError( + prerequisite_feature=feature_name, + condition_json_path=condition_json_path, + ) SegmentFlagReference.objects.bulk_create( SegmentFlagReference( segment=segment, diff --git a/api/segments/serializers.py b/api/segments/serializers.py index c1984e1414f8..63954ccfa6bb 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -218,8 +218,9 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: def create(self, validated_data: dict[str, Any]): # type: ignore[no-untyped-def] metadata_data = validated_data.pop("metadata", []) self._set_rules_data(validated_data) - segment = super().create(validated_data) # type: ignore[no-untyped-call] - index_segment_flag_references(segment) + with transaction.atomic(): + segment = super().create(validated_data) # type: ignore[no-untyped-call] + index_segment_flag_references(segment) self._update_metadata(segment, metadata_data) enqueue_membership_refresh(segment.project) return segment From b37eac57000e69d98ea2e20512fc5c9dc8d4d312 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Tue, 15 Sep 2026 19:24:23 -0300 Subject: [PATCH 04/21] unindex --- api/features/dependencies/services.py | 34 +++++++++++++-- api/segments/services.py | 63 +++++++++++++++------------ 2 files changed, 67 insertions(+), 30 deletions(-) diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 07b1dadb2e22..47332f9096e3 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -1,18 +1,25 @@ +import typing + import jsonpath_rfc9535 +import structlog from jsonpath_rfc9535.exceptions import JSONPathError from jsonpath_rfc9535.segments import JSONPathChildSegment, JSONPathSegment from jsonpath_rfc9535.selectors import NameSelector from features.dependencies.exceptions import PrerequisiteFeatureNotFoundError from features.dependencies.models import SegmentFlagReference -from features.models import Feature -from segments.models import Segment +from features.models import Feature, FeatureSegment from segments.types import SegmentRule +if typing.TYPE_CHECKING: + from segments.models import Segment + +logger = structlog.get_logger(__name__) + JSONPathStr = str -def index_segment_flag_references(segment: Segment) -> None: +def index_segment_flag_references(segment: "Segment") -> None: """Materialise the segment's `$.flags` conditions as SegmentFlagReference rows.""" feature_names_by_json_path = { f"{rule_json_path}.conditions[{condition_index}]": feature_name @@ -45,6 +52,27 @@ def index_segment_flag_references(segment: Segment) -> None: ) +def delete_segment_flag_references(segment: "Segment") -> None: + """Drop the segment's index rows, reporting every dependency lost.""" + references = SegmentFlagReference.objects.filter(segment=segment).select_related( + "prerequisite_feature" + ) + overrides = FeatureSegment.objects.filter(segment=segment).select_related( + "environment", "feature" + ) + for override in overrides: + for reference in references: + logger.info( + "dependencies.deleted", + organisation__id=segment.project.organisation_id, + project__id=segment.project_id, + environment__key=override.environment.api_key, + feature__name=override.feature.name, + prerequisite_feature__name=reference.prerequisite_feature.name, + ) + references.delete() + + def _get_rules_by_json_path( rules: list[SegmentRule], ) -> dict[JSONPathStr, SegmentRule]: diff --git a/api/segments/services.py b/api/segments/services.py index c7518cf46963..492a46300f28 100644 --- a/api/segments/services.py +++ b/api/segments/services.py @@ -1,11 +1,13 @@ import typing import uuid +from datetime import datetime from django.db import models, transaction from django.db.models import QuerySet from django.utils import timezone from core.dataclasses import AuthorData +from features.dependencies.services import delete_segment_flag_references from features.models import FeatureSegment from features.versioning.models import EnvironmentFeatureVersion @@ -50,17 +52,9 @@ def delete_segment( segment: "Segment", author: AuthorData, ) -> None: - """ - Delete a segment using optimized bulk operations. - - Uses bulk UPDATE/DELETE operations instead of individual soft-deletes, - reducing the number of database queries from O(n) to O(1) where n is - the number of rules and conditions. - - TODO: Delete as per https://github.com/Flagsmith/flagsmith/issues/7818 - """ + """Delete a segment and all of its components""" from features.models import FeatureSegment - from segments.models import Condition, Segment, SegmentRule + from segments.models import Segment from segments.tasks import create_segment_deleted_audit_log now = timezone.now() @@ -76,6 +70,36 @@ def delete_segment( ).values_list("id", flat=True) ) + with transaction.atomic(): + delete_segment_flag_references(segment) + FeatureSegment.objects.filter(segment_id__in=segment_ids).delete() + _delete_legacy_rules_and_conditions(segment_ids, now) + Segment.objects.filter(id__in=segment_ids).update(deleted_at=now) + + create_segment_deleted_audit_log.delay( + args=( + project_id, + segment_name, + segment_id, + segment_uuid, + author.user.id if author.user else None, + author.api_key.id if author.api_key else None, + now.isoformat(), + ) + ) + + +def _delete_legacy_rules_and_conditions( + segment_ids: list[int], + now: datetime, +) -> None: + """Delete a segment's rules and conditions (rows) using bulk operations + + This is only needed until `Segment.rules_data` becomes the source of truth. + TODO: Delete as per https://github.com/Flagsmith/flagsmith/issues/7818 + """ + from segments.models import Condition, SegmentRule + top_level_rule_ids = list( SegmentRule.objects.filter(segment_id__in=segment_ids).values_list( "id", flat=True @@ -96,23 +120,8 @@ def delete_segment( all_rule_ids_list = list(all_rule_ids) - with transaction.atomic(): - FeatureSegment.objects.filter(segment_id__in=segment_ids).delete() - Condition.objects.filter(rule_id__in=all_rule_ids_list).update(deleted_at=now) - SegmentRule.objects.filter(id__in=all_rule_ids_list).update(deleted_at=now) - Segment.objects.filter(id__in=segment_ids).update(deleted_at=now) - - create_segment_deleted_audit_log.delay( - args=( - project_id, - segment_name, - segment_id, - segment_uuid, - author.user.id if author.user else None, - author.api_key.id if author.api_key else None, - now.isoformat(), - ) - ) + Condition.objects.filter(rule_id__in=all_rule_ids_list).update(deleted_at=now) + SegmentRule.objects.filter(id__in=all_rule_ids_list).update(deleted_at=now) def copy_segment_rules_and_conditions( From c40259976690ce60e71c51ef29fcd84d538613e8 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Tue, 15 Sep 2026 20:10:03 -0300 Subject: [PATCH 05/21] reindex --- api/features/dependencies/services.py | 28 +++++++++++++++++++ api/segments/serializers.py | 1 + .../observability/_events-catalogue.md | 27 +++++++++++++++++- 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 47332f9096e3..5b458c00510a 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -42,6 +42,11 @@ def index_segment_flag_references(segment: "Segment") -> None: prerequisite_feature=feature_name, condition_json_path=condition_json_path, ) + references = SegmentFlagReference.objects.filter(segment=segment) + previous_feature_names = set( + references.values_list("prerequisite_feature__name", flat=True) + ) + references.delete() SegmentFlagReference.objects.bulk_create( SegmentFlagReference( segment=segment, @@ -50,6 +55,29 @@ def index_segment_flag_references(segment: "Segment") -> None: ) for condition_json_path, feature_name in feature_names_by_json_path.items() ) + overrides = FeatureSegment.objects.filter(segment=segment).select_related( + "environment", "feature" + ) + for feature_name in previous_feature_names - feature_ids_by_name.keys(): + for override in overrides: + logger.info( + "dependencies.deleted", + organisation__id=segment.project.organisation_id, + project__id=segment.project_id, + environment__key=override.environment.api_key, + feature__name=override.feature.name, + prerequisite_feature__name=feature_name, + ) + for feature_name in feature_ids_by_name.keys() - previous_feature_names: + for override in overrides: + logger.info( + "dependencies.created", + organisation__id=segment.project.organisation_id, + project__id=segment.project_id, + environment__key=override.environment.api_key, + feature__name=override.feature.name, + prerequisite_feature__name=feature_name, + ) def delete_segment_flag_references(segment: "Segment") -> None: diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 63954ccfa6bb..1c07478396d6 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -237,6 +237,7 @@ def update(self, segment: Segment, validated_data: dict[str, Any]): # type: ign revision_id=segment_revision.id, ) segment = super().update(segment, validated_data) # type: ignore[no-untyped-call] + index_segment_flag_references(segment) self._update_metadata(segment, metadata) enqueue_membership_refresh(segment.project) return segment diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 3826b5823873..bf17bd6523f7 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -325,6 +325,31 @@ Attributes: - `environment.id` - `organisation.id` +### `features.dependencies.services.dependencies.created` + +Logged at `info` from: + - `api/features/dependencies/services.py:73` + +Attributes: + - `environment.key` + - `feature.name` + - `organisation.id` + - `prerequisite_feature.name` + - `project.id` + +### `features.dependencies.services.dependencies.deleted` + +Logged at `info` from: + - `api/features/dependencies/services.py:63` + - `api/features/dependencies/services.py:93` + +Attributes: + - `environment.key` + - `feature.name` + - `organisation.id` + - `prerequisite_feature.name` + - `project.id` + ### `features.flag.update_rejected` Logged at `warning` from: @@ -706,7 +731,7 @@ Attributes: ### `segments.serializers.segment_revision_created` Logged at `info` from: - - `api/segments/serializers.py:231` + - `api/segments/serializers.py:234` Attributes: - `revision_id` From 7d5c5040e2c25989d849ffd600aae0858e8ab06c Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Tue, 15 Sep 2026 20:10:28 -0300 Subject: [PATCH 06/21] don't touch the hen --- .../dependencies/test_segment_management.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index 30dcc5f44a69..b5adcda058b7 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -147,12 +147,18 @@ def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( chicken = Feature.objects.create(name="chicken", project_id=project) egg = Feature.objects.create(name="egg", project_id=project) rooster = Feature.objects.create(name="rooster", project_id=project) + hen = Feature.objects.create(name="hen", project_id=project) segment = Segment.objects.create(name="segment", project_id=project) SegmentFlagReference.objects.create( segment=segment, prerequisite_feature=egg, condition_json_path="$[0].conditions[0]", ) + SegmentFlagReference.objects.create( + segment=segment, + prerequisite_feature=hen, + condition_json_path="$[0].conditions[1]", + ) FeatureSegment.objects.create( segment=segment, feature=chicken, @@ -174,6 +180,11 @@ def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( "operator": "EQUAL", "value": True, }, + { + "property": "$.flags.hen.enabled", + "operator": "EQUAL", + "value": True, + }, ], } ], @@ -195,7 +206,14 @@ def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( "prerequisite_feature": rooster.id, "condition_json_path": "$[0].conditions[0]", }, + { + "segment": segment.id, + "prerequisite_feature": hen.id, + "condition_json_path": "$[0].conditions[1]", + }, ] + assert not log.has("dependencies.deleted", prerequisite_feature__name="hen") + assert not log.has("dependencies.created", prerequisite_feature__name="hen") assert log.has( "dependencies.deleted", level="info", From c5ccaa27a794601b28de84909d0934403c3677ea Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Tue, 15 Sep 2026 20:12:12 -0300 Subject: [PATCH 07/21] fix logging --- api/features/dependencies/services.py | 2 +- .../observability/_events-catalogue.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 5b458c00510a..324688e1529c 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -14,7 +14,7 @@ if typing.TYPE_CHECKING: from segments.models import Segment -logger = structlog.get_logger(__name__) +logger = structlog.get_logger("features") JSONPathStr = str diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index bf17bd6523f7..49f2c2e919a6 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -325,7 +325,7 @@ Attributes: - `environment.id` - `organisation.id` -### `features.dependencies.services.dependencies.created` +### `features.dependencies.created` Logged at `info` from: - `api/features/dependencies/services.py:73` @@ -337,7 +337,7 @@ Attributes: - `prerequisite_feature.name` - `project.id` -### `features.dependencies.services.dependencies.deleted` +### `features.dependencies.deleted` Logged at `info` from: - `api/features/dependencies/services.py:63` From 3608b5ecff90f7e9b5bf0d3a2dfa77649f676c58 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Tue, 15 Sep 2026 21:20:23 -0300 Subject: [PATCH 08/21] or maybe the egg --- api/features/dependencies/exceptions.py | 23 +++++++ api/features/dependencies/services.py | 69 ++++++++++++++++++- api/features/dependencies/types.py | 20 ++++++ api/features/views.py | 7 +- api/segments/serializers.py | 6 +- api/segments/services.py | 2 +- .../observability/_events-catalogue.md | 20 ++++-- 7 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 api/features/dependencies/types.py diff --git a/api/features/dependencies/exceptions.py b/api/features/dependencies/exceptions.py index a3647056312b..410e2d1d8326 100644 --- a/api/features/dependencies/exceptions.py +++ b/api/features/dependencies/exceptions.py @@ -1,6 +1,29 @@ +from typing import TypedDict + from rest_framework import status from rest_framework.exceptions import APIException +from features.dependencies.types import DependencyPath + + +class _CircularDependencyDetail(TypedDict): + """The body served where a feature is refused for depending on itself.""" + + code: str + path: DependencyPath + + +class CircularDependencyError(APIException): + """Raised where a feature would end up depending on itself.""" + + status_code = status.HTTP_400_BAD_REQUEST + default_code = "circular_dependency" + + def __init__(self, path: DependencyPath) -> None: + super().__init__() + detail: _CircularDependencyDetail = {"code": self.default_code, "path": path} + self.detail = detail # type: ignore[assignment] + class PrerequisiteFeatureNotFoundError(APIException): """Raised where a segment condition names a feature that does not exist.""" diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 324688e1529c..9e4d0bd619ad 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -1,4 +1,5 @@ import typing +from collections import defaultdict import jsonpath_rfc9535 import structlog @@ -6,9 +7,14 @@ from jsonpath_rfc9535.segments import JSONPathChildSegment, JSONPathSegment from jsonpath_rfc9535.selectors import NameSelector -from features.dependencies.exceptions import PrerequisiteFeatureNotFoundError +from features.dependencies.exceptions import ( + CircularDependencyError, + PrerequisiteFeatureNotFoundError, +) from features.dependencies.models import SegmentFlagReference +from features.dependencies.types import DependencyEdge, DependencyPath from features.models import Feature, FeatureSegment +from segments.services import get_overrides_in_effect from segments.types import SegmentRule if typing.TYPE_CHECKING: @@ -101,6 +107,67 @@ def delete_segment_flag_references(segment: "Segment") -> None: references.delete() +def validate_segment_flag_dependencies(segment: "Segment") -> None: + """Raise if any feature the segment overrides ends up depending on itself.""" + for override in ( + get_overrides_in_effect() + .filter(segment=segment) + .select_related("environment", "feature") + ): + edges: dict[str, list[DependencyEdge]] = defaultdict(list) + for ( + feature_name, + prerequisite_feature_name, + segment_id, + segment_name, + condition_json_path, + ) in ( + get_overrides_in_effect() + .filter( + environment=override.environment, + segment__flag_references__isnull=False, + ) + .values_list( + "feature__name", + "segment__flag_references__prerequisite_feature__name", + "segment_id", + "segment__name", + "segment__flag_references__condition_json_path", + ) + ): + edges[feature_name].append( + { + "feature": feature_name, + "needs": prerequisite_feature_name, + "segment": { + "id": segment_id, + "name": segment_name, + "condition_json_path": condition_json_path, + }, + } + ) + pending: list[DependencyPath] = [ + [edge] for edge in edges[override.feature.name] + ] + visited: set[str] = set() + while pending: + path = pending.pop() + if (prerequisite_feature_name := path[-1]["needs"]) in visited: + continue + if prerequisite_feature_name == override.feature.name: + logger.info( + "dependencies.create_failed", + organisation__id=segment.project.organisation_id, + project__id=segment.project_id, + environment__key=override.environment.api_key, + feature__name=override.feature.name, + prerequisite_feature__name=path[0]["needs"], + ) + raise CircularDependencyError(path=path) + visited.add(prerequisite_feature_name) + pending += [[*path, edge] for edge in edges[prerequisite_feature_name]] + + def _get_rules_by_json_path( rules: list[SegmentRule], ) -> dict[JSONPathStr, SegmentRule]: diff --git a/api/features/dependencies/types.py b/api/features/dependencies/types.py new file mode 100644 index 000000000000..304b2340c649 --- /dev/null +++ b/api/features/dependencies/types.py @@ -0,0 +1,20 @@ +import typing + + +class ReferencingSegment(typing.TypedDict): + """The segment whose rules hold the `$.flags` condition making up a dependency.""" + + id: int + name: str + condition_json_path: str + + +class DependencyEdge(typing.TypedDict): + """One feature's dependency on another.""" + + feature: str + needs: str + segment: ReferencingSegment + + +DependencyPath = list[DependencyEdge] diff --git a/api/features/views.py b/api/features/views.py index 9ddf9f6b590e..db62e6f445f1 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -7,6 +7,7 @@ from common.projects.permissions import VIEW_PROJECT from django.conf import settings from django.core.cache import caches +from django.db import transaction from django.db.models import ( BooleanField, Case, @@ -62,6 +63,7 @@ EnvironmentKeyPermissions, NestedEnvironmentPermissions, ) +from features.dependencies.services import validate_segment_flag_dependencies from features.feature_lifecycle.services import ( annotate_feature_queryset_with_lifecycle_stage, is_feature_lifecycle_enabled, @@ -1221,5 +1223,8 @@ def create_segment_override( # type: ignore[no-untyped-def] data=request.data, context={"environment": environment, "feature": feature} ) serializer.is_valid(raise_exception=True) - serializer.save(environment=environment, feature=feature) # type: ignore[no-untyped-call] + with transaction.atomic(): + feature_state = serializer.save(environment=environment, feature=feature) # type: ignore[no-untyped-call] + if feature_segment := feature_state.feature_segment: + validate_segment_flag_dependencies(feature_segment.segment) return Response(serializer.data, status=201) diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 1c07478396d6..809a7d6bda87 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -10,7 +10,10 @@ from cohorts.models import Cohort from edge_api.utils import is_edge_enabled -from features.dependencies.services import index_segment_flag_references +from features.dependencies.services import ( + index_segment_flag_references, + validate_segment_flag_dependencies, +) from metadata.serializers import MetadataSerializer, MetadataSerializerMixin from projects.models import Project from segment_membership.constants import MAX_SEGMENT_MEMBERS_PAGE_SIZE @@ -238,6 +241,7 @@ def update(self, segment: Segment, validated_data: dict[str, Any]): # type: ign ) segment = super().update(segment, validated_data) # type: ignore[no-untyped-call] index_segment_flag_references(segment) + validate_segment_flag_dependencies(segment) self._update_metadata(segment, metadata) enqueue_membership_refresh(segment.project) return segment diff --git a/api/segments/services.py b/api/segments/services.py index 492a46300f28..f55ed48f9e17 100644 --- a/api/segments/services.py +++ b/api/segments/services.py @@ -7,7 +7,6 @@ from django.utils import timezone from core.dataclasses import AuthorData -from features.dependencies.services import delete_segment_flag_references from features.models import FeatureSegment from features.versioning.models import EnvironmentFeatureVersion @@ -53,6 +52,7 @@ def delete_segment( author: AuthorData, ) -> None: """Delete a segment and all of its components""" + from features.dependencies.services import delete_segment_flag_references from features.models import FeatureSegment from segments.models import Segment from segments.tasks import create_segment_deleted_audit_log diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 49f2c2e919a6..57f71d6bb121 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -325,10 +325,22 @@ Attributes: - `environment.id` - `organisation.id` +### `features.dependencies.create_failed` + +Logged at `info` from: + - `api/features/dependencies/services.py:158` + +Attributes: + - `environment.key` + - `feature.name` + - `organisation.id` + - `prerequisite_feature.name` + - `project.id` + ### `features.dependencies.created` Logged at `info` from: - - `api/features/dependencies/services.py:73` + - `api/features/dependencies/services.py:79` Attributes: - `environment.key` @@ -340,8 +352,8 @@ Attributes: ### `features.dependencies.deleted` Logged at `info` from: - - `api/features/dependencies/services.py:63` - - `api/features/dependencies/services.py:93` + - `api/features/dependencies/services.py:69` + - `api/features/dependencies/services.py:99` Attributes: - `environment.key` @@ -731,7 +743,7 @@ Attributes: ### `segments.serializers.segment_revision_created` Logged at `info` from: - - `api/segments/serializers.py:234` + - `api/segments/serializers.py:237` Attributes: - `revision_id` From 640bffdaf73360fadaa93ffc37ff62ce383084d9 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Tue, 15 Sep 2026 22:09:15 -0300 Subject: [PATCH 09/21] more queries! --- api/tests/unit/segments/test_unit_segments_services.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/tests/unit/segments/test_unit_segments_services.py b/api/tests/unit/segments/test_unit_segments_services.py index c6ff3289e58c..e3eff1f94fab 100644 --- a/api/tests/unit/segments/test_unit_segments_services.py +++ b/api/tests/unit/segments/test_unit_segments_services.py @@ -182,8 +182,8 @@ def test_delete_segment__varying_segment_sizes__query_count_is_constant( large_query_count = len(ctx_large.captured_queries) # Then - # 11 for the delete, 15 for the audit log task (runs synchronously in tests) - assert small_query_count == large_query_count == 26 + # 13 for the delete, 15 for the audit log task (runs synchronously in tests) + assert small_query_count == large_query_count == 28 # TODO: Delete as per https://github.com/Flagsmith/flagsmith/issues/7818 From 219a407a32b95a2110646921b15bb404395c2f5c Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 14:52:38 -0300 Subject: [PATCH 10/21] feature segments --- api/features/feature_segments/views.py | 12 +++ .../dependencies/test_segment_management.py | 77 +++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/api/features/feature_segments/views.py b/api/features/feature_segments/views.py index a44d24a0f587..d761c190d840 100644 --- a/api/features/feature_segments/views.py +++ b/api/features/feature_segments/views.py @@ -1,6 +1,7 @@ import logging from common.projects.permissions import VIEW_PROJECT +from django.db import transaction from django.utils.decorators import method_decorator from drf_spectacular.utils import extend_schema from rest_framework import viewsets @@ -9,6 +10,7 @@ from rest_framework.response import Response from environments.models import Environment +from features.dependencies.services import validate_segment_flag_dependencies from features.feature_segments.serializers import ( FeatureSegmentChangePrioritiesSerializer, FeatureSegmentCreateSerializer, @@ -79,6 +81,16 @@ def get_queryset(self): # type: ignore[no-untyped-def] return queryset + @transaction.atomic + def perform_create(self, serializer: FeatureSegmentCreateSerializer) -> None: + super().perform_create(serializer) + validate_segment_flag_dependencies(serializer.instance.segment) + + @transaction.atomic + def perform_update(self, serializer: FeatureSegmentCreateSerializer) -> None: + super().perform_update(serializer) + validate_segment_flag_dependencies(serializer.instance.segment) + def get_serializer_class(self): # type: ignore[no-untyped-def] if self.action in ["create", "update", "partial_update"]: return FeatureSegmentCreateSerializer diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index b5adcda058b7..13b563a2021a 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -648,3 +648,80 @@ def test_update_segment_update_rules__longer_dependency_cycle_path__responds_400 feature__name="rooster", prerequisite_feature__name="chicken", ) + + +def test_create_feature_segment__circular_flag_dependency__responds_400( + admin_client: APIClient, + environment: int, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + chicken_segment = Segment.objects.create(name="chicken_segment", project_id=project) + egg_segment = Segment.objects.create(name="egg_segment", project_id=project) + SegmentFlagReference.objects.create( + segment=chicken_segment, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[1]", + ) + FeatureSegment.objects.create( + segment=chicken_segment, + feature=chicken, + environment_id=environment, + ) + SegmentFlagReference.objects.create( + segment=egg_segment, + prerequisite_feature=chicken, + condition_json_path="$[1].conditions[2]", + ) + + # When + response = admin_client.post( + "/api/v1/features/feature-segments/", + data={ + "feature": egg.id, + "segment": egg_segment.id, + "environment": environment, + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "code": "circular_dependency", + "path": [ + { + "feature": "egg", + "needs": "chicken", + "segment": { + "id": egg_segment.id, + "name": egg_segment.name, + "condition_json_path": "$[1].conditions[2]", + }, + }, + { + "feature": "chicken", + "needs": "egg", + "segment": { + "id": chicken_segment.id, + "name": chicken_segment.name, + "condition_json_path": "$[0].conditions[1]", + }, + }, + ], + } + assert not FeatureSegment.objects.filter(segment=egg_segment, feature=egg).exists() + assert log.has( + "dependencies.create_failed", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="egg", + prerequisite_feature__name="chicken", + ) From b7eb928455b94f6d55152b798ff51c7846234753 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 15:09:36 -0300 Subject: [PATCH 11/21] =?UTF-8?q?coverage=20=F0=9F=92=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/features/dependencies/mappers.py | 56 +++++++++++++++++++ api/features/dependencies/services.py | 56 ++----------------- api/features/dependencies/types.py | 3 + .../unit/features/dependencies/__init__.py | 0 .../features/dependencies/test_mappers.py | 40 +++++++++++++ .../observability/_events-catalogue.md | 8 +-- 6 files changed, 107 insertions(+), 56 deletions(-) create mode 100644 api/features/dependencies/mappers.py create mode 100644 api/tests/unit/features/dependencies/__init__.py create mode 100644 api/tests/unit/features/dependencies/test_mappers.py diff --git a/api/features/dependencies/mappers.py b/api/features/dependencies/mappers.py new file mode 100644 index 000000000000..0d469a22b459 --- /dev/null +++ b/api/features/dependencies/mappers.py @@ -0,0 +1,56 @@ +import jsonpath_rfc9535 +from jsonpath_rfc9535.exceptions import JSONPathError +from jsonpath_rfc9535.segments import JSONPathChildSegment, JSONPathSegment +from jsonpath_rfc9535.selectors import NameSelector + +from features.dependencies.types import FeatureName, JSONPathStr +from segments.types import SegmentRule + + +def map_rules_to_prerequisite_feature_names( + rules: list[SegmentRule], +) -> dict[JSONPathStr, FeatureName]: + """Returns the feature names keyed by the condition $.flags JSONPath""" + return { + f"{rule_json_path}.conditions[{condition_index}]": feature_name + for rule_json_path, rule in _get_rules_by_json_path(rules).items() + for condition_index, condition in enumerate(rule["conditions"]) + if (condition_property := condition["property"]) + and (feature_name := _get_prerequisite_feature_name(condition_property)) + } + + +def _get_rules_by_json_path( + rules: list[SegmentRule], +) -> dict[JSONPathStr, SegmentRule]: + rules_by_json_path: dict[JSONPathStr, SegmentRule] = {} + for rule_index, rule in enumerate(rules): + rule_json_path = f"$[{rule_index}]" + rules_by_json_path[rule_json_path] = rule + for nested_index, nested_rule in enumerate(rule.get("rules", [])): + rules_by_json_path[f"{rule_json_path}.rules[{nested_index}]"] = nested_rule + return rules_by_json_path + + +def _get_prerequisite_feature_name(condition_property: str) -> FeatureName | None: + """Return the feature name a `$.flags.` condition points at, if it does.""" + # Because of historical decisions, `$['flags']` can be a trait. + if not condition_property.startswith("$.flags"): + return None + try: + query_segments = jsonpath_rfc9535.compile(condition_property).segments + except JSONPathError: + return None + if len(query_segments) < 2 or _get_selected_name(query_segments[0]) != "flags": + return None + return _get_selected_name(query_segments[1]) + + +def _get_selected_name(query_segment: JSONPathSegment) -> str | None: + if ( + not isinstance(query_segment, JSONPathChildSegment) + or len(query_segment.selectors) != 1 + ): + return None + selector = query_segment.selectors[0] + return selector.name if isinstance(selector, NameSelector) else None diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 9e4d0bd619ad..959d5d376769 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -1,41 +1,29 @@ import typing from collections import defaultdict -import jsonpath_rfc9535 import structlog -from jsonpath_rfc9535.exceptions import JSONPathError -from jsonpath_rfc9535.segments import JSONPathChildSegment, JSONPathSegment -from jsonpath_rfc9535.selectors import NameSelector from features.dependencies.exceptions import ( CircularDependencyError, PrerequisiteFeatureNotFoundError, ) +from features.dependencies.mappers import map_rules_to_prerequisite_feature_names from features.dependencies.models import SegmentFlagReference from features.dependencies.types import DependencyEdge, DependencyPath from features.models import Feature, FeatureSegment from segments.services import get_overrides_in_effect -from segments.types import SegmentRule if typing.TYPE_CHECKING: from segments.models import Segment logger = structlog.get_logger("features") -JSONPathStr = str - def index_segment_flag_references(segment: "Segment") -> None: """Materialise the segment's `$.flags` conditions as SegmentFlagReference rows.""" - feature_names_by_json_path = { - f"{rule_json_path}.conditions[{condition_index}]": feature_name - for rule_json_path, rule in _get_rules_by_json_path( - segment.rules_data or [] - ).items() - for condition_index, condition in enumerate(rule["conditions"]) - if (condition_property := condition["property"]) - and (feature_name := _get_prerequisite_feature_name(condition_property)) - } + feature_names_by_json_path = map_rules_to_prerequisite_feature_names( + segment.rules_data or [] + ) feature_ids_by_name = dict( Feature.objects.filter( project_id=segment.project_id, @@ -166,39 +154,3 @@ def validate_segment_flag_dependencies(segment: "Segment") -> None: raise CircularDependencyError(path=path) visited.add(prerequisite_feature_name) pending += [[*path, edge] for edge in edges[prerequisite_feature_name]] - - -def _get_rules_by_json_path( - rules: list[SegmentRule], -) -> dict[JSONPathStr, SegmentRule]: - rules_by_json_path: dict[JSONPathStr, SegmentRule] = {} - for rule_index, rule in enumerate(rules): - rule_json_path = f"$[{rule_index}]" - rules_by_json_path[rule_json_path] = rule - for nested_index, nested_rule in enumerate(rule.get("rules", [])): - rules_by_json_path[f"{rule_json_path}.rules[{nested_index}]"] = nested_rule - return rules_by_json_path - - -def _get_prerequisite_feature_name(condition_property: str) -> str | None: - """Return the feature name a `$.flags.` condition points at, if it does.""" - # Because of historical decisions, `$['flags']` can be a trait. - if not condition_property.startswith("$.flags"): - return None - try: - query_segments = jsonpath_rfc9535.compile(condition_property).segments - except JSONPathError: - return None - if len(query_segments) < 2 or _get_selected_name(query_segments[0]) != "flags": - return None - return _get_selected_name(query_segments[1]) - - -def _get_selected_name(query_segment: JSONPathSegment) -> str | None: - if ( - not isinstance(query_segment, JSONPathChildSegment) - or len(query_segment.selectors) != 1 - ): - return None - selector = query_segment.selectors[0] - return selector.name if isinstance(selector, NameSelector) else None diff --git a/api/features/dependencies/types.py b/api/features/dependencies/types.py index 304b2340c649..2b7b624e6624 100644 --- a/api/features/dependencies/types.py +++ b/api/features/dependencies/types.py @@ -1,5 +1,8 @@ import typing +FeatureName = str +JSONPathStr = str + class ReferencingSegment(typing.TypedDict): """The segment whose rules hold the `$.flags` condition making up a dependency.""" diff --git a/api/tests/unit/features/dependencies/__init__.py b/api/tests/unit/features/dependencies/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/api/tests/unit/features/dependencies/test_mappers.py b/api/tests/unit/features/dependencies/test_mappers.py new file mode 100644 index 000000000000..1ff222affbfc --- /dev/null +++ b/api/tests/unit/features/dependencies/test_mappers.py @@ -0,0 +1,40 @@ +import pytest + +from features.dependencies.mappers import map_rules_to_prerequisite_feature_names +from segments.types import SegmentRule + + +@pytest.mark.parametrize( + "condition_property", + [ + "$.flags['unterminated", + "$.flags['flag_a','flag_b'].enabled", + "$.flags..enabled", + "$.traits.flags.enabled", + "$.flagsmith.enabled", + "$.flags", + ], +) +def test_map_rules_to_prerequisite_feature_names__invalid_prerequisite_jsonpath__ignores_property( + condition_property: str, +) -> None: + # Given + rules: list[SegmentRule] = [ + { + "type": "ALL", + "conditions": [ + { + "property": condition_property, + "operator": "EQUAL", + "value": "true", + "description": None, + } + ], + } + ] + + # When + feature_names_by_json_path = map_rules_to_prerequisite_feature_names(rules) + + # Then + assert feature_names_by_json_path == {} diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 57f71d6bb121..c6440f75c34b 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -328,7 +328,7 @@ Attributes: ### `features.dependencies.create_failed` Logged at `info` from: - - `api/features/dependencies/services.py:158` + - `api/features/dependencies/services.py:146` Attributes: - `environment.key` @@ -340,7 +340,7 @@ Attributes: ### `features.dependencies.created` Logged at `info` from: - - `api/features/dependencies/services.py:79` + - `api/features/dependencies/services.py:67` Attributes: - `environment.key` @@ -352,8 +352,8 @@ Attributes: ### `features.dependencies.deleted` Logged at `info` from: - - `api/features/dependencies/services.py:69` - - `api/features/dependencies/services.py:99` + - `api/features/dependencies/services.py:57` + - `api/features/dependencies/services.py:87` Attributes: - `environment.key` From d0df9a40762aa69b845a50297a6d3ac7d563c750 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 15:10:21 -0300 Subject: [PATCH 12/21] =?UTF-8?q?uma=20quest=C3=A3o=20de=20ordem=20?= =?UTF-8?q?=F0=9F=A7=91=E2=80=8D=E2=9A=96=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../features/dependencies/test_segment_management.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index 13b563a2021a..3a1fc3c29bd5 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -74,7 +74,7 @@ def test_create_segment__valid_flag_dependency__indexes_created( assert response.status_code == 201 segment_id = response.json()["id"] assert list( - SegmentFlagReference.objects.values( + SegmentFlagReference.objects.order_by("condition_json_path").values( "segment", "prerequisite_feature", "condition_json_path", @@ -195,7 +195,7 @@ def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( # Then assert response.status_code == 200 assert list( - SegmentFlagReference.objects.values( + SegmentFlagReference.objects.order_by("condition_json_path").values( "segment", "prerequisite_feature", "condition_json_path", From 3e9efe048a51c8d020f32488646e3ea83e0d6f04 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 17:45:49 -0300 Subject: [PATCH 13/21] prepare for the future --- api/features/dependencies/services.py | 56 +++--- api/features/future/services.py | 79 ++++++-- api/tests/integration/conftest.py | 28 +++ .../dependencies/test_segment_management.py | 187 ++++++++++++++---- api/tests/types.py | 10 + .../observability/_events-catalogue.md | 12 +- 6 files changed, 280 insertions(+), 92 deletions(-) diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 959d5d376769..0ee08202800b 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -1,15 +1,17 @@ import typing from collections import defaultdict +from collections.abc import Collection import structlog +from environments.models import Environment from features.dependencies.exceptions import ( CircularDependencyError, PrerequisiteFeatureNotFoundError, ) from features.dependencies.mappers import map_rules_to_prerequisite_feature_names from features.dependencies.models import SegmentFlagReference -from features.dependencies.types import DependencyEdge, DependencyPath +from features.dependencies.types import DependencyEdge, DependencyPath, FeatureName from features.models import Feature, FeatureSegment from segments.services import get_overrides_in_effect @@ -49,29 +51,15 @@ def index_segment_flag_references(segment: "Segment") -> None: ) for condition_json_path, feature_name in feature_names_by_json_path.items() ) - overrides = FeatureSegment.objects.filter(segment=segment).select_related( - "environment", "feature" - ) - for feature_name in previous_feature_names - feature_ids_by_name.keys(): - for override in overrides: - logger.info( - "dependencies.deleted", - organisation__id=segment.project.organisation_id, - project__id=segment.project_id, - environment__key=override.environment.api_key, - feature__name=override.feature.name, - prerequisite_feature__name=feature_name, - ) - for feature_name in feature_ids_by_name.keys() - previous_feature_names: - for override in overrides: - logger.info( - "dependencies.created", - organisation__id=segment.project.organisation_id, - project__id=segment.project_id, - environment__key=override.environment.api_key, - feature__name=override.feature.name, - prerequisite_feature__name=feature_name, - ) + for override in FeatureSegment.objects.filter(segment=segment).select_related( + "environment__project", "feature" + ): + report_flag_dependencies( + environment=override.environment, + feature=override.feature, + created=feature_ids_by_name.keys() - previous_feature_names, + deleted=previous_feature_names - feature_ids_by_name.keys(), + ) def delete_segment_flag_references(segment: "Segment") -> None: @@ -95,6 +83,26 @@ def delete_segment_flag_references(segment: "Segment") -> None: references.delete() +def report_flag_dependencies( + *, + environment: Environment, + feature: Feature, + created: Collection[FeatureName], + deleted: Collection[FeatureName], +) -> None: + """Report the prerequisites a feature gains and loses in an environment.""" + log = logger.bind( + organisation__id=environment.project.organisation_id, + project__id=environment.project_id, + environment__key=environment.api_key, + feature__name=feature.name, + ) + for feature_name in deleted: + log.info("dependencies.deleted", prerequisite_feature__name=feature_name) + for feature_name in created: + log.info("dependencies.created", prerequisite_feature__name=feature_name) + + def validate_segment_flag_dependencies(segment: "Segment") -> None: """Raise if any feature the segment overrides ends up depending on itself.""" for override in ( diff --git a/api/features/future/services.py b/api/features/future/services.py index d2fe64aed62c..3ab180a6f3a7 100644 --- a/api/features/future/services.py +++ b/api/features/future/services.py @@ -9,6 +9,12 @@ from api_keys.user import APIKeyUser from environments.models import Environment +from features.dependencies.models import SegmentFlagReference +from features.dependencies.services import ( + report_flag_dependencies, + validate_segment_flag_dependencies, +) +from features.dependencies.types import FeatureName from features.future.exceptions import ( DuplicatePriorityError, SegmentOverrideNotFoundError, @@ -29,11 +35,18 @@ from features.multivariate.models import MultivariateFeatureStateValue from features.versioning.models import EnvironmentFeatureVersion from features.versioning.versioning_service import get_environment_flags_list +from segments.models import Segment from users.models import FFAdminUser logger = structlog.get_logger("features") +class _OverriddenSegments(NamedTuple): + created: list[int] + updated: list[int] + deleted: list[int] + + def _get_feature_states( environment: Environment, feature: Feature ) -> list[FeatureState]: @@ -234,12 +247,6 @@ def _check_priorities( raise DuplicatePriorityError(f"Duplicate priority: {duplicate}.") -class WrittenSegmentOverrides(NamedTuple): - created: list[int] - updated: list[int] - deleted: list[int] - - def _write_segment_overrides( *, environment: Environment, @@ -249,24 +256,24 @@ def _write_segment_overrides( overrides: dict[int, FeatureState], changes: Sequence[SegmentOverrideRequest], replace: bool, -) -> WrittenSegmentOverrides: - written = WrittenSegmentOverrides([], [], []) +) -> _OverriddenSegments: + segments = _OverriddenSegments([], [], []) if replace: - written.deleted.extend( + segments.deleted.extend( sorted(overrides.keys() - {change["segment"]["id"] for change in changes}) ) _delete_segment_overrides( environment=environment, feature=feature, version=version, - segment_ids=written.deleted, + segment_ids=segments.deleted, ) for position, change in enumerate(changes): segment_id = change["segment"]["id"] if feature_state := overrides.get(segment_id): - written.updated.append(segment_id) + segments.updated.append(segment_id) else: feature_state = _create_segment_override( environment=environment, @@ -275,17 +282,31 @@ def _write_segment_overrides( segment_id=segment_id, priority=change.get("priority", position), ) - written.created.append(segment_id) + segments.created.append(segment_id) _write_segment_override( feature_state, change, - replace=replace or segment_id in written.created, + replace=replace or segment_id in segments.created, environment_default=environment_default, ) _check_priorities(environment, feature, version) - return written + return segments + + +def _get_prerequisite_feature_names( + segments: _OverriddenSegments, +) -> tuple[set[FeatureName], set[FeatureName]]: + """Return the prerequisite feature names the flag gains and loses.""" + created: set[FeatureName] = set() + deleted: set[FeatureName] = set() + for segment_id, feature_name in SegmentFlagReference.objects.filter( + segment_id__in=[*segments.created, *segments.deleted] + ).values_list("segment_id", "prerequisite_feature__name"): + names = created if segment_id in segments.created else deleted + names.add(feature_name) + return created, deleted def update_flag( @@ -301,7 +322,7 @@ def update_flag( if writes_nothing: return get_flag(environment=environment, feature=feature) - written = WrittenSegmentOverrides([], [], []) + segments = _OverriddenSegments([], [], []) with transaction.atomic(): version = _create_draft_version(environment, feature) @@ -318,7 +339,7 @@ def update_flag( ) if (override_changes := changes.get("segment_overrides")) is not None: - written = _write_segment_overrides( + segments = _write_segment_overrides( environment=environment, feature=feature, version=version, @@ -331,15 +352,25 @@ def update_flag( if version is not None: _publish_version(version, author) + for segment in Segment.objects.filter(id__in=segments.created): + validate_segment_flag_dependencies(segment) + created, deleted = _get_prerequisite_feature_names(segments) + report_flag_dependencies( + environment=environment, + feature=feature, + created=created, + deleted=deleted, + ) + logger.info( "flag.updated", organisation__id=environment.project.organisation_id, project__id=environment.project_id, environment__id=environment.id, feature__id=feature.id, - segment_overrides__created__segment__ids=written.created, - segment_overrides__updated__segment__ids=written.updated, - segment_overrides__deleted__segment__ids=written.deleted, + segment_overrides__created__segment__ids=segments.created, + segment_overrides__updated__segment__ids=segments.updated, + segment_overrides__deleted__segment__ids=segments.deleted, ) return get_flag(environment=environment, feature=feature) @@ -370,6 +401,16 @@ def delete_segment_override( if version is not None: _publish_version(version, author) + _, deleted = _get_prerequisite_feature_names( + _OverriddenSegments([], [], [segment_id]) + ) + report_flag_dependencies( + environment=environment, + feature=feature, + created=[], + deleted=deleted, + ) + logger.info( "flag.updated", organisation__id=environment.project.organisation_id, diff --git a/api/tests/integration/conftest.py b/api/tests/integration/conftest.py index ac6174e4d7e6..bb16fd2025a7 100644 --- a/api/tests/integration/conftest.py +++ b/api/tests/integration/conftest.py @@ -15,8 +15,10 @@ from app.utils import create_hash from app_analytics.influxdb_wrapper import InfluxDBWrapper from environments.enums import EnvironmentDocumentCacheMode +from features.future.types import UpdateFlagRequest from organisations.models import Organisation from tests.integration.helpers import create_mv_option_with_api +from tests.types import CreateSegmentOverrideFixture from users.models import FFAdminUser @@ -385,6 +387,32 @@ def feature_segment(admin_client, segment, feature, environment): # type: ignor return response.json()["id"] +@pytest.fixture() +def create_segment_override(admin_client: APIClient) -> CreateSegmentOverrideFixture: + """Return a callable putting a segment override live, whichever versioning is in use.""" + + def _create_segment_override( + environment_api_key: str, + feature_id: int, + segment_id: int, + enabled: bool = True, + ) -> None: + response = admin_client.patch( + f"/api/__future__/environments/{environment_api_key}/features/{feature_id}/", + UpdateFlagRequest( + { + "segment_overrides": [ + {"segment": {"id": segment_id}, "enabled": enabled} + ] + } + ), + format="json", + ) + assert response.status_code == status.HTTP_200_OK + + return _create_segment_override + + @pytest.fixture() def segment_featurestate( admin_client: APIClient, diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index 3a1fc3c29bd5..3d94f725046f 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -3,9 +3,11 @@ from rest_framework.test import APIClient from features.dependencies.models import SegmentFlagReference +from features.future.types import UpdateFlagRequest from features.models import Feature, FeatureSegment from projects.models import Project from segments.models import Segment +from tests.types import CreateSegmentOverrideFixture @pytest.fixture( @@ -135,6 +137,45 @@ def test_create_segment__nonexistent_prerequisite__responds_400( assert not SegmentFlagReference.objects.exists() +def test_update_segment_update_rules__nonexistent_prerequisite__responds_400( + admin_client: APIClient, + project: int, +) -> None: + # Given + segment = Segment.objects.create(name="segment", project_id=project) + + # When + response = admin_client.put( + f"/api/v1/projects/{project}/segments/{segment.id}/", + data={ + "name": "segment", + "project": project, + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.unicorn.enabled", + "operator": "EQUAL", + "value": True, + }, + ], + } + ], + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "code": "prerequisite_feature_not_found", + "prerequisite_feature": "unicorn", + "condition_json_path": "$[0].conditions[0]", + } + assert not SegmentFlagReference.objects.exists() + + def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( admin_client: APIClient, environment: int, @@ -354,10 +395,9 @@ def test_update_segment_add_override__circular_flag_dependency__responds_400( ) -@pytest.mark.usefixtures("with_and_without_segment_change_requests") -def test_update_segment_add_override__longer_dependency_cycle_path__responds_400( +@pytest.mark.usefixtures("versioned_environment") +def test_update_flag_add_override__flag_dependency__reports_dependency_created( admin_client: APIClient, - environment: int, environment_api_key: str, log: StructuredLogCapture, organisation: int, @@ -366,44 +406,114 @@ def test_update_segment_add_override__longer_dependency_cycle_path__responds_400 # Given chicken = Feature.objects.create(name="chicken", project_id=project) egg = Feature.objects.create(name="egg", project_id=project) - rooster = Feature.objects.create(name="rooster", project_id=project) - segment1 = Segment.objects.create(name="segment1", project_id=project) - segment2 = Segment.objects.create(name="segment2", project_id=project) - segment3 = Segment.objects.create(name="segment3", project_id=project) + segment = Segment.objects.create(name="segment", project_id=project) SegmentFlagReference.objects.create( - segment=segment1, + segment=segment, prerequisite_feature=egg, condition_json_path="$[0].conditions[0]", ) - FeatureSegment.objects.create( - segment=segment1, - feature=chicken, - environment_id=environment, + + # When + response = admin_client.patch( + f"/api/__future__/environments/{environment_api_key}/features/{chicken.id}/", + UpdateFlagRequest( + {"segment_overrides": [{"segment": {"id": segment.id}, "enabled": True}]} + ), + format="json", + ) + + # Then + assert response.status_code == 200 + assert log.has( + "dependencies.created", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="chicken", + prerequisite_feature__name="egg", ) + + +@pytest.mark.usefixtures("versioned_environment") +def test_delete_override__flag_dependency__reports_dependency_deleted( + admin_client: APIClient, + create_segment_override: CreateSegmentOverrideFixture, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + segment = Segment.objects.create(name="segment", project_id=project) SegmentFlagReference.objects.create( - segment=segment2, - prerequisite_feature=rooster, + segment=segment, + prerequisite_feature=egg, condition_json_path="$[0].conditions[0]", ) - FeatureSegment.objects.create( - segment=segment2, - feature=egg, - environment_id=environment, + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=segment.id, + ) + + # When + response = admin_client.delete( + f"/api/__future__/environments/{environment_api_key}" + f"/features/{chicken.id}/segment-overrides/{segment.id}/", + ) + + # Then + assert response.status_code == 200 + assert log.has( + "dependencies.deleted", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="chicken", + prerequisite_feature__name="egg", + ) + + +@pytest.mark.usefixtures("versioned_environment") +def test_update_flag_add_override__circular_flag_dependency__responds_400( + admin_client: APIClient, + create_segment_override: CreateSegmentOverrideFixture, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + segment1 = Segment.objects.create(name="segment1", project_id=project) + segment2 = Segment.objects.create(name="segment2", project_id=project) + SegmentFlagReference.objects.create( + segment=segment1, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[1]", + ) + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=segment1.id, ) SegmentFlagReference.objects.create( - segment=segment3, + segment=segment2, prerequisite_feature=chicken, - condition_json_path="$[0].conditions[0]", + condition_json_path="$[1].conditions[2]", ) # When - response = admin_client.post( - f"/api/v1/environments/{environment_api_key}/features/{rooster.id}/create-segment-override/", - data={ - "feature_state_value": {}, - "feature_segment": {"segment": segment3.id}, - "enabled": True, - }, + response = admin_client.patch( + f"/api/__future__/environments/{environment_api_key}/features/{egg.id}/", + UpdateFlagRequest( + {"segment_overrides": [{"segment": {"id": segment2.id}, "enabled": True}]} + ), format="json", ) @@ -413,12 +523,12 @@ def test_update_segment_add_override__longer_dependency_cycle_path__responds_400 "code": "circular_dependency", "path": [ { - "feature": "rooster", + "feature": "egg", "needs": "chicken", "segment": { - "id": segment3.id, - "name": segment3.name, - "condition_json_path": "$[0].conditions[0]", + "id": segment2.id, + "name": segment2.name, + "condition_json_path": "$[1].conditions[2]", }, }, { @@ -427,28 +537,19 @@ def test_update_segment_add_override__longer_dependency_cycle_path__responds_400 "segment": { "id": segment1.id, "name": segment1.name, - "condition_json_path": "$[0].conditions[0]", - }, - }, - { - "feature": "egg", - "needs": "rooster", - "segment": { - "id": segment2.id, - "name": segment2.name, - "condition_json_path": "$[0].conditions[0]", + "condition_json_path": "$[0].conditions[1]", }, }, ], } - assert not FeatureSegment.objects.filter(segment=segment3, feature=rooster).exists() + assert not FeatureSegment.objects.filter(segment=segment2, feature=egg).exists() assert log.has( "dependencies.create_failed", level="info", organisation__id=organisation, project__id=project, environment__key=environment_api_key, - feature__name="rooster", + feature__name="egg", prerequisite_feature__name="chicken", ) diff --git a/api/tests/types.py b/api/tests/types.py index 4547c3fbb146..7ef079855b21 100644 --- a/api/tests/types.py +++ b/api/tests/types.py @@ -44,3 +44,13 @@ def __call__(self, *feature_names: str) -> None: ... class MigratorFactory(Protocol): def __call__(self, name: Optional[str] = None) -> Migrator: ... + + +class CreateSegmentOverrideFixture(Protocol): + def __call__( + self, + environment_api_key: str, + feature_id: int, + segment_id: int, + enabled: bool = True, + ) -> None: ... diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index c6440f75c34b..aec83d27d7a4 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -328,7 +328,7 @@ Attributes: ### `features.dependencies.create_failed` Logged at `info` from: - - `api/features/dependencies/services.py:146` + - `api/features/dependencies/services.py:154` Attributes: - `environment.key` @@ -340,7 +340,7 @@ Attributes: ### `features.dependencies.created` Logged at `info` from: - - `api/features/dependencies/services.py:67` + - `api/features/dependencies/services.py:103` Attributes: - `environment.key` @@ -352,8 +352,8 @@ Attributes: ### `features.dependencies.deleted` Logged at `info` from: - - `api/features/dependencies/services.py:57` - - `api/features/dependencies/services.py:87` + - `api/features/dependencies/services.py:75` + - `api/features/dependencies/services.py:101` Attributes: - `environment.key` @@ -377,8 +377,8 @@ Attributes: ### `features.flag.updated` Logged at `info` from: - - `api/features/future/services.py:334` - - `api/features/future/services.py:373` + - `api/features/future/services.py:365` + - `api/features/future/services.py:404` Attributes: - `environment.id` From 6f54c4de3e0bac00c665d7f4d1256d3b4593a7a0 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 20:13:53 -0300 Subject: [PATCH 14/21] feature versioning v2 strikes again --- api/tests/integration/features/conftest.py | 36 ++++++ .../dependencies/test_segment_management.py | 103 ++++++++---------- .../features/future/test_flag_endpoint.py | 12 +- 3 files changed, 84 insertions(+), 67 deletions(-) create mode 100644 api/tests/integration/features/conftest.py diff --git a/api/tests/integration/features/conftest.py b/api/tests/integration/features/conftest.py new file mode 100644 index 000000000000..89e995c231fd --- /dev/null +++ b/api/tests/integration/features/conftest.py @@ -0,0 +1,36 @@ +import pytest +from rest_framework.test import APIClient + +from environments.models import Environment +from features.versioning.tasks import enable_v2_versioning + + +@pytest.fixture() +def other_environment( + admin_client: APIClient, + project: int, + versioned_environment: Environment, +) -> Environment: + """A second environment in the project, versioned like the first one.""" + response = admin_client.post( + "/api/v1/environments/", + data={"name": "Other Environment", "project": project}, + format="json", + ) + assert response.status_code == 201 + environment: Environment = Environment.objects.get( + api_key=response.json()["api_key"] + ) + if versioned_environment.use_v2_feature_versioning: + enable_v2_versioning(environment_id=environment.id) + return environment + + +@pytest.fixture(params=["feature_versioning_v1", "feature_versioning_v2"]) +def versioned_environment( + request: pytest.FixtureRequest, + environment: int, +) -> Environment: + if request.param == "feature_versioning_v2": + enable_v2_versioning(environment_id=environment) + return Environment.objects.get(id=environment) # type: ignore[no-any-return] diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index 3d94f725046f..a37c7ec82516 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -5,26 +5,10 @@ from features.dependencies.models import SegmentFlagReference from features.future.types import UpdateFlagRequest from features.models import Feature, FeatureSegment -from projects.models import Project from segments.models import Segment from tests.types import CreateSegmentOverrideFixture -@pytest.fixture( - params=[False, True], - ids=["change_requests_disabled", "change_requests_enabled"], -) -def with_and_without_segment_change_requests( - request: pytest.FixtureRequest, - project: int, -) -> bool: - enabled: bool = request.param - if enabled: - Project.objects.filter(id=project).update(minimum_change_request_approvals=0) - return enabled - - -@pytest.mark.usefixtures("with_and_without_segment_change_requests") def test_create_segment__valid_flag_dependency__indexes_created( admin_client: APIClient, project: int, @@ -100,7 +84,6 @@ def test_create_segment__valid_flag_dependency__indexes_created( ] -@pytest.mark.usefixtures("with_and_without_segment_change_requests") def test_create_segment__nonexistent_prerequisite__responds_400( admin_client: APIClient, project: int, @@ -176,9 +159,10 @@ def test_update_segment_update_rules__nonexistent_prerequisite__responds_400( assert not SegmentFlagReference.objects.exists() +@pytest.mark.usefixtures("versioned_environment") def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( admin_client: APIClient, - environment: int, + create_segment_override: CreateSegmentOverrideFixture, environment_api_key: str, log: StructuredLogCapture, organisation: int, @@ -190,6 +174,11 @@ def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( rooster = Feature.objects.create(name="rooster", project_id=project) hen = Feature.objects.create(name="hen", project_id=project) segment = Segment.objects.create(name="segment", project_id=project) + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=segment.id, + ) SegmentFlagReference.objects.create( segment=segment, prerequisite_feature=egg, @@ -200,11 +189,6 @@ def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( prerequisite_feature=hen, condition_json_path="$[0].conditions[1]", ) - FeatureSegment.objects.create( - segment=segment, - feature=chicken, - environment_id=environment, - ) # When response = admin_client.put( @@ -275,9 +259,10 @@ def test_update_segment_update_rules__valid_flag_dependency__indexes_updated( ) +@pytest.mark.usefixtures("versioned_environment") def test_delete_segment__flag_dependency__indexes_removed( admin_client: APIClient, - environment: int, + create_segment_override: CreateSegmentOverrideFixture, environment_api_key: str, log: StructuredLogCapture, organisation: int, @@ -287,16 +272,16 @@ def test_delete_segment__flag_dependency__indexes_removed( chicken = Feature.objects.create(name="chicken", project_id=project) egg = Feature.objects.create(name="egg", project_id=project) segment = Segment.objects.create(name="segment", project_id=project) + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=segment.id, + ) SegmentFlagReference.objects.create( segment=segment, prerequisite_feature=egg, condition_json_path="$[0].conditions[0]", ) - FeatureSegment.objects.create( - segment=segment, - feature=chicken, - environment_id=environment, - ) # When response = admin_client.delete( @@ -317,7 +302,6 @@ def test_delete_segment__flag_dependency__indexes_removed( ) -@pytest.mark.usefixtures("with_and_without_segment_change_requests") def test_update_segment_add_override__circular_flag_dependency__responds_400( admin_client: APIClient, environment: int, @@ -554,9 +538,10 @@ def test_update_flag_add_override__circular_flag_dependency__responds_400( ) +@pytest.mark.usefixtures("versioned_environment") def test_update_segment_update_rules__circular_flag_dependency__responds_400( admin_client: APIClient, - environment: int, + create_segment_override: CreateSegmentOverrideFixture, environment_api_key: str, log: StructuredLogCapture, organisation: int, @@ -567,21 +552,21 @@ def test_update_segment_update_rules__circular_flag_dependency__responds_400( egg = Feature.objects.create(name="egg", project_id=project) segment1 = Segment.objects.create(name="segment1", project_id=project) segment2 = Segment.objects.create(name="segment2", project_id=project) + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=segment1.id, + ) + create_segment_override( + environment_api_key=environment_api_key, + feature_id=egg.id, + segment_id=segment2.id, + ) SegmentFlagReference.objects.create( segment=segment1, prerequisite_feature=egg, condition_json_path="$[0].conditions[1]", ) - FeatureSegment.objects.create( - segment=segment1, - feature=chicken, - environment_id=environment, - ) - FeatureSegment.objects.create( - segment=segment2, - feature=egg, - environment_id=environment, - ) # When response = admin_client.put( @@ -642,9 +627,10 @@ def test_update_segment_update_rules__circular_flag_dependency__responds_400( ) +@pytest.mark.usefixtures("versioned_environment") def test_update_segment_update_rules__longer_dependency_cycle_path__responds_400( admin_client: APIClient, - environment: int, + create_segment_override: CreateSegmentOverrideFixture, environment_api_key: str, log: StructuredLogCapture, organisation: int, @@ -662,25 +648,25 @@ def test_update_segment_update_rules__longer_dependency_cycle_path__responds_400 prerequisite_feature=egg, condition_json_path="$[0].conditions[0]", ) - FeatureSegment.objects.create( - segment=segment1, - feature=chicken, - environment_id=environment, + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=segment1.id, ) SegmentFlagReference.objects.create( segment=segment2, prerequisite_feature=rooster, condition_json_path="$[0].conditions[0]", ) - FeatureSegment.objects.create( - segment=segment2, - feature=egg, - environment_id=environment, + create_segment_override( + environment_api_key=environment_api_key, + feature_id=egg.id, + segment_id=segment2.id, ) - FeatureSegment.objects.create( - segment=segment3, - feature=rooster, - environment_id=environment, + create_segment_override( + environment_api_key=environment_api_key, + feature_id=rooster.id, + segment_id=segment3.id, ) # When @@ -753,6 +739,7 @@ def test_update_segment_update_rules__longer_dependency_cycle_path__responds_400 def test_create_feature_segment__circular_flag_dependency__responds_400( admin_client: APIClient, + create_segment_override: CreateSegmentOverrideFixture, environment: int, environment_api_key: str, log: StructuredLogCapture, @@ -769,10 +756,10 @@ def test_create_feature_segment__circular_flag_dependency__responds_400( prerequisite_feature=egg, condition_json_path="$[0].conditions[1]", ) - FeatureSegment.objects.create( - segment=chicken_segment, - feature=chicken, - environment_id=environment, + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=chicken_segment.id, ) SegmentFlagReference.objects.create( segment=egg_segment, diff --git a/api/tests/integration/features/future/test_flag_endpoint.py b/api/tests/integration/features/future/test_flag_endpoint.py index 39af42a6b4cd..d1aa6e3a0bd6 100644 --- a/api/tests/integration/features/future/test_flag_endpoint.py +++ b/api/tests/integration/features/future/test_flag_endpoint.py @@ -12,21 +12,15 @@ from features.future.types import UpdateFlagRequest from features.models import FeatureState from features.versioning.models import EnvironmentFeatureVersion -from features.versioning.tasks import enable_v2_versioning from organisations.models import Organisation from tests.integration.helpers import create_mv_option_with_api from tests.types import WithEnvironmentPermissionsCallable from users.models import FFAdminUser -@pytest.fixture(params=["feature_versioning_v1", "feature_versioning_v2"], autouse=True) -def versioned_environment( - request: pytest.FixtureRequest, - environment: int, -) -> Environment: - if request.param == "feature_versioning_v2": - enable_v2_versioning(environment_id=environment) - return Environment.objects.get(id=environment) # type: ignore[no-any-return] +@pytest.fixture(autouse=True) +def versioned_environment(versioned_environment: Environment) -> Environment: + return versioned_environment @pytest.fixture() From eae3ff834307ae1d39062e8929ad1abdd0ffa4d3 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 20:25:36 -0300 Subject: [PATCH 15/21] even cycles needs an environment --- api/features/dependencies/exceptions.py | 13 ++- api/features/dependencies/services.py | 8 +- api/features/dependencies/types.py | 7 ++ .../dependencies/test_segment_management.py | 110 ++++++++++++++++++ 4 files changed, 134 insertions(+), 4 deletions(-) diff --git a/api/features/dependencies/exceptions.py b/api/features/dependencies/exceptions.py index 410e2d1d8326..f83ac2749fc9 100644 --- a/api/features/dependencies/exceptions.py +++ b/api/features/dependencies/exceptions.py @@ -3,13 +3,14 @@ from rest_framework import status from rest_framework.exceptions import APIException -from features.dependencies.types import DependencyPath +from features.dependencies.types import DependencyPath, ReferencingEnvironment class _CircularDependencyDetail(TypedDict): """The body served where a feature is refused for depending on itself.""" code: str + environment: ReferencingEnvironment path: DependencyPath @@ -19,9 +20,15 @@ class CircularDependencyError(APIException): status_code = status.HTTP_400_BAD_REQUEST default_code = "circular_dependency" - def __init__(self, path: DependencyPath) -> None: + def __init__( + self, environment: ReferencingEnvironment, path: DependencyPath + ) -> None: super().__init__() - detail: _CircularDependencyDetail = {"code": self.default_code, "path": path} + detail: _CircularDependencyDetail = { + "code": self.default_code, + "environment": environment, + "path": path, + } self.detail = detail # type: ignore[assignment] diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 0ee08202800b..9a3ee2d24e4f 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -159,6 +159,12 @@ def validate_segment_flag_dependencies(segment: "Segment") -> None: feature__name=override.feature.name, prerequisite_feature__name=path[0]["needs"], ) - raise CircularDependencyError(path=path) + raise CircularDependencyError( + environment={ + "key": override.environment.api_key, + "name": override.environment.name, + }, + path=path, + ) visited.add(prerequisite_feature_name) pending += [[*path, edge] for edge in edges[prerequisite_feature_name]] diff --git a/api/features/dependencies/types.py b/api/features/dependencies/types.py index 2b7b624e6624..05e2083573d2 100644 --- a/api/features/dependencies/types.py +++ b/api/features/dependencies/types.py @@ -12,6 +12,13 @@ class ReferencingSegment(typing.TypedDict): condition_json_path: str +class ReferencingEnvironment(typing.TypedDict): + """The environment whose live overrides make up a dependency graph.""" + + key: str + name: str + + class DependencyEdge(typing.TypedDict): """One feature's dependency on another.""" diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index a37c7ec82516..3f308e0c4c46 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -2,6 +2,7 @@ from pytest_structlog import StructuredLogCapture from rest_framework.test import APIClient +from environments.models import Environment from features.dependencies.models import SegmentFlagReference from features.future.types import UpdateFlagRequest from features.models import Feature, FeatureSegment @@ -306,6 +307,7 @@ def test_update_segment_add_override__circular_flag_dependency__responds_400( admin_client: APIClient, environment: int, environment_api_key: str, + environment_name: str, log: StructuredLogCapture, organisation: int, project: int, @@ -346,6 +348,7 @@ def test_update_segment_add_override__circular_flag_dependency__responds_400( assert response.status_code == 400 assert response.json() == { "code": "circular_dependency", + "environment": {"key": environment_api_key, "name": environment_name}, "path": [ { "feature": "egg", @@ -467,6 +470,7 @@ def test_update_flag_add_override__circular_flag_dependency__responds_400( admin_client: APIClient, create_segment_override: CreateSegmentOverrideFixture, environment_api_key: str, + environment_name: str, log: StructuredLogCapture, organisation: int, project: int, @@ -505,6 +509,7 @@ def test_update_flag_add_override__circular_flag_dependency__responds_400( assert response.status_code == 400 assert response.json() == { "code": "circular_dependency", + "environment": {"key": environment_api_key, "name": environment_name}, "path": [ { "feature": "egg", @@ -543,6 +548,7 @@ def test_update_segment_update_rules__circular_flag_dependency__responds_400( admin_client: APIClient, create_segment_override: CreateSegmentOverrideFixture, environment_api_key: str, + environment_name: str, log: StructuredLogCapture, organisation: int, project: int, @@ -594,6 +600,7 @@ def test_update_segment_update_rules__circular_flag_dependency__responds_400( assert response.status_code == 400 assert response.json() == { "code": "circular_dependency", + "environment": {"key": environment_api_key, "name": environment_name}, "path": [ { "feature": "egg", @@ -627,11 +634,111 @@ def test_update_segment_update_rules__circular_flag_dependency__responds_400( ) +@pytest.mark.usefixtures("versioned_environment") +def test_update_segment_update_rules__cycle_in_another_environment__responds_400( + admin_client: APIClient, + create_segment_override: CreateSegmentOverrideFixture, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + other_environment: Environment, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + segment1 = Segment.objects.create(name="segment1", project_id=project) + segment2 = Segment.objects.create(name="segment2", project_id=project) + SegmentFlagReference.objects.create( + segment=segment1, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[0]", + ) + create_segment_override( + environment_api_key=environment_api_key, + feature_id=egg.id, + segment_id=segment2.id, + ) + create_segment_override( + environment_api_key=other_environment.api_key, + feature_id=egg.id, + segment_id=segment2.id, + ) + create_segment_override( + environment_api_key=other_environment.api_key, + feature_id=chicken.id, + segment_id=segment1.id, + ) + + # When + response = admin_client.put( + f"/api/v1/projects/{project}/segments/{segment2.id}/", + data={ + "name": "segment2", + "project": project, + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.chicken.enabled", + "operator": "EQUAL", + "value": True, + }, + ], + } + ], + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "code": "circular_dependency", + "environment": { + "key": other_environment.api_key, + "name": other_environment.name, + }, + "path": [ + { + "feature": "egg", + "needs": "chicken", + "segment": { + "id": segment2.id, + "name": segment2.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + { + "feature": "chicken", + "needs": "egg", + "segment": { + "id": segment1.id, + "name": segment1.name, + "condition_json_path": "$[0].conditions[0]", + }, + }, + ], + } + assert not SegmentFlagReference.objects.filter(segment=segment2).exists() + assert log.has( + "dependencies.create_failed", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=other_environment.api_key, + feature__name="egg", + prerequisite_feature__name="chicken", + ) + + @pytest.mark.usefixtures("versioned_environment") def test_update_segment_update_rules__longer_dependency_cycle_path__responds_400( admin_client: APIClient, create_segment_override: CreateSegmentOverrideFixture, environment_api_key: str, + environment_name: str, log: StructuredLogCapture, organisation: int, project: int, @@ -695,6 +802,7 @@ def test_update_segment_update_rules__longer_dependency_cycle_path__responds_400 assert response.status_code == 400 assert response.json() == { "code": "circular_dependency", + "environment": {"key": environment_api_key, "name": environment_name}, "path": [ { "feature": "rooster", @@ -742,6 +850,7 @@ def test_create_feature_segment__circular_flag_dependency__responds_400( create_segment_override: CreateSegmentOverrideFixture, environment: int, environment_api_key: str, + environment_name: str, log: StructuredLogCapture, organisation: int, project: int, @@ -782,6 +891,7 @@ def test_create_feature_segment__circular_flag_dependency__responds_400( assert response.status_code == 400 assert response.json() == { "code": "circular_dependency", + "environment": {"key": environment_api_key, "name": environment_name}, "path": [ { "feature": "egg", From 175dbe6a8bf9488e70ea1bc3a8d09fb4a5b3bd0d Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 20:47:09 -0300 Subject: [PATCH 16/21] typing grrr --- api/features/feature_segments/views.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/api/features/feature_segments/views.py b/api/features/feature_segments/views.py index d761c190d840..7a8cd2b0ab37 100644 --- a/api/features/feature_segments/views.py +++ b/api/features/feature_segments/views.py @@ -8,6 +8,7 @@ from rest_framework.decorators import action from rest_framework.generics import get_object_or_404 from rest_framework.response import Response +from rest_framework.serializers import BaseSerializer from environments.models import Environment from features.dependencies.services import validate_segment_flag_dependencies @@ -82,14 +83,14 @@ def get_queryset(self): # type: ignore[no-untyped-def] return queryset @transaction.atomic - def perform_create(self, serializer: FeatureSegmentCreateSerializer) -> None: - super().perform_create(serializer) - validate_segment_flag_dependencies(serializer.instance.segment) + def perform_create(self, serializer: BaseSerializer[FeatureSegment]) -> None: + feature_segment = serializer.save() + validate_segment_flag_dependencies(feature_segment.segment) @transaction.atomic - def perform_update(self, serializer: FeatureSegmentCreateSerializer) -> None: - super().perform_update(serializer) - validate_segment_flag_dependencies(serializer.instance.segment) + def perform_update(self, serializer: BaseSerializer[FeatureSegment]) -> None: + feature_segment = serializer.save() + validate_segment_flag_dependencies(feature_segment.segment) def get_serializer_class(self): # type: ignore[no-untyped-def] if self.action in ["create", "update", "partial_update"]: From 8e2728ad5d3a291b3d823964442bf991c1ec9ffc Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 21:40:30 -0300 Subject: [PATCH 17/21] =?UTF-8?q?coverage=20=F0=9F=92=AF=20part=202?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/features/feature_segments/views.py | 5 -- api/tests/integration/conftest.py | 16 ++-- .../dependencies/test_segment_management.py | 77 +++++++++++++++++++ api/tests/types.py | 1 + .../observability/_events-catalogue.md | 2 +- 5 files changed, 87 insertions(+), 14 deletions(-) diff --git a/api/features/feature_segments/views.py b/api/features/feature_segments/views.py index 7a8cd2b0ab37..9dabc59b2abb 100644 --- a/api/features/feature_segments/views.py +++ b/api/features/feature_segments/views.py @@ -87,11 +87,6 @@ def perform_create(self, serializer: BaseSerializer[FeatureSegment]) -> None: feature_segment = serializer.save() validate_segment_flag_dependencies(feature_segment.segment) - @transaction.atomic - def perform_update(self, serializer: BaseSerializer[FeatureSegment]) -> None: - feature_segment = serializer.save() - validate_segment_flag_dependencies(feature_segment.segment) - def get_serializer_class(self): # type: ignore[no-untyped-def] if self.action in ["create", "update", "partial_update"]: return FeatureSegmentCreateSerializer diff --git a/api/tests/integration/conftest.py b/api/tests/integration/conftest.py index bb16fd2025a7..76c7f47ed18d 100644 --- a/api/tests/integration/conftest.py +++ b/api/tests/integration/conftest.py @@ -15,7 +15,7 @@ from app.utils import create_hash from app_analytics.influxdb_wrapper import InfluxDBWrapper from environments.enums import EnvironmentDocumentCacheMode -from features.future.types import UpdateFlagRequest +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 @@ -396,16 +396,16 @@ def _create_segment_override( feature_id: int, segment_id: int, enabled: bool = True, + priority: int | None = None, ) -> None: + segment_override = SegmentOverrideRequest( + {"segment": {"id": segment_id}, "enabled": enabled} + ) + if priority is not None: + segment_override["priority"] = priority response = admin_client.patch( f"/api/__future__/environments/{environment_api_key}/features/{feature_id}/", - UpdateFlagRequest( - { - "segment_overrides": [ - {"segment": {"id": segment_id}, "enabled": enabled} - ] - } - ), + UpdateFlagRequest({"segment_overrides": [segment_override]}), format="json", ) assert response.status_code == status.HTTP_200_OK diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index 3f308e0c4c46..f97b6dbff41a 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -845,6 +845,83 @@ def test_update_segment_update_rules__longer_dependency_cycle_path__responds_400 ) +@pytest.mark.usefixtures("versioned_environment") +def test_update_segment_update_rules__shared_prerequisite__responds_200( + admin_client: APIClient, + create_segment_override: CreateSegmentOverrideFixture, + environment_api_key: str, + log: StructuredLogCapture, + organisation: int, + project: int, +) -> None: + # Given + chicken = Feature.objects.create(name="chicken", project_id=project) + egg = Feature.objects.create(name="egg", project_id=project) + other_segment = Segment.objects.create(name="other_segment", project_id=project) + segment = Segment.objects.create(name="segment", project_id=project) + SegmentFlagReference.objects.create( + segment=other_segment, + prerequisite_feature=egg, + condition_json_path="$[0].conditions[0]", + ) + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=other_segment.id, + priority=0, + ) + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=segment.id, + priority=1, + ) + + # When + response = admin_client.put( + f"/api/v1/projects/{project}/segments/{segment.id}/", + data={ + "name": "segment", + "project": project, + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.egg.enabled", + "operator": "EQUAL", + "value": True, + }, + ], + } + ], + }, + format="json", + ) + + # Then + assert response.status_code == 200 + assert list( + SegmentFlagReference.objects.filter(segment=segment).values( + "prerequisite_feature", "condition_json_path" + ) + ) == [ + { + "prerequisite_feature": egg.id, + "condition_json_path": "$[0].conditions[0]", + }, + ] + assert log.has( + "dependencies.created", + level="info", + organisation__id=organisation, + project__id=project, + environment__key=environment_api_key, + feature__name="chicken", + prerequisite_feature__name="egg", + ) + + def test_create_feature_segment__circular_flag_dependency__responds_400( admin_client: APIClient, create_segment_override: CreateSegmentOverrideFixture, diff --git a/api/tests/types.py b/api/tests/types.py index 7ef079855b21..ffbfd955bda5 100644 --- a/api/tests/types.py +++ b/api/tests/types.py @@ -53,4 +53,5 @@ def __call__( feature_id: int, segment_id: int, enabled: bool = True, + priority: int | None = None, ) -> None: ... diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index f54605c9d4e8..1d8f8320a65b 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -322,7 +322,7 @@ Attributes: Logged at `info` from: - `api/features/future/services.py:365` - - `api/features/future/services.py:404` + - `api/features/future/services.py:414` Attributes: - `environment.id` From b25eecd833f8c2ef95b4c3792e39fec96fd29d4b Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Wed, 16 Sep 2026 22:10:14 -0300 Subject: [PATCH 18/21] can't depend on a mystery --- api/features/dependencies/mappers.py | 1 + .../dependencies/test_segment_management.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/api/features/dependencies/mappers.py b/api/features/dependencies/mappers.py index 0d469a22b459..c965301c291f 100644 --- a/api/features/dependencies/mappers.py +++ b/api/features/dependencies/mappers.py @@ -17,6 +17,7 @@ def map_rules_to_prerequisite_feature_names( for condition_index, condition in enumerate(rule["conditions"]) if (condition_property := condition["property"]) and (feature_name := _get_prerequisite_feature_name(condition_property)) + is not None } diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index f97b6dbff41a..af3ae81ca3c3 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -85,9 +85,18 @@ def test_create_segment__valid_flag_dependency__indexes_created( ] +@pytest.mark.parametrize( + ("property", "prerequisite_feature"), + [ + ("$.flags.unicorn.enabled", "unicorn"), + ("$.flags[''].enabled", ""), + ], +) def test_create_segment__nonexistent_prerequisite__responds_400( admin_client: APIClient, project: int, + property: str, + prerequisite_feature: str, ) -> None: # Given / When response = admin_client.post( @@ -99,7 +108,7 @@ def test_create_segment__nonexistent_prerequisite__responds_400( "type": "ALL", "conditions": [ { - "property": "$.flags.unicorn.enabled", + "property": property, "operator": "EQUAL", "value": True, }, @@ -114,7 +123,7 @@ def test_create_segment__nonexistent_prerequisite__responds_400( assert response.status_code == 400 assert response.json() == { "code": "prerequisite_feature_not_found", - "prerequisite_feature": "unicorn", + "prerequisite_feature": prerequisite_feature, "condition_json_path": "$[0].conditions[0]", } assert not Segment.objects.exists() From ed3decf344f474d466d9954a0ac44dacb097ad17 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Fri, 18 Sep 2026 09:18:05 -0300 Subject: [PATCH 19/21] less hops --- api/features/dependencies/services.py | 74 +++++++++++-------- .../features/dependencies/test_services.py | 38 ++++++++++ .../observability/_events-catalogue.md | 2 +- 3 files changed, 82 insertions(+), 32 deletions(-) create mode 100644 api/tests/unit/features/dependencies/test_services.py diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 9a3ee2d24e4f..b79a1c442633 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -105,43 +105,17 @@ def report_flag_dependencies( def validate_segment_flag_dependencies(segment: "Segment") -> None: """Raise if any feature the segment overrides ends up depending on itself.""" + edges_by_environment_id: dict[int, dict[FeatureName, list[DependencyEdge]]] = {} for override in ( get_overrides_in_effect() .filter(segment=segment) .select_related("environment", "feature") ): - edges: dict[str, list[DependencyEdge]] = defaultdict(list) - for ( - feature_name, - prerequisite_feature_name, - segment_id, - segment_name, - condition_json_path, - ) in ( - get_overrides_in_effect() - .filter( - environment=override.environment, - segment__flag_references__isnull=False, - ) - .values_list( - "feature__name", - "segment__flag_references__prerequisite_feature__name", - "segment_id", - "segment__name", - "segment__flag_references__condition_json_path", - ) - ): - edges[feature_name].append( - { - "feature": feature_name, - "needs": prerequisite_feature_name, - "segment": { - "id": segment_id, - "name": segment_name, - "condition_json_path": condition_json_path, - }, - } + if override.environment_id not in edges_by_environment_id: + edges_by_environment_id[override.environment_id] = _get_dependency_edges( + override.environment ) + edges = edges_by_environment_id[override.environment_id] pending: list[DependencyPath] = [ [edge] for edge in edges[override.feature.name] ] @@ -168,3 +142,41 @@ def validate_segment_flag_dependencies(segment: "Segment") -> None: ) visited.add(prerequisite_feature_name) pending += [[*path, edge] for edge in edges[prerequisite_feature_name]] + + +def _get_dependency_edges( + environment: Environment, +) -> dict[FeatureName, list[DependencyEdge]]: + edges: dict[FeatureName, list[DependencyEdge]] = defaultdict(list) + for ( + feature_name, + prerequisite_feature_name, + segment_id, + segment_name, + condition_json_path, + ) in ( + get_overrides_in_effect() + .filter( + environment=environment, + segment__flag_references__isnull=False, + ) + .values_list( + "feature__name", + "segment__flag_references__prerequisite_feature__name", + "segment_id", + "segment__name", + "segment__flag_references__condition_json_path", + ) + ): + edges[feature_name].append( + { + "feature": feature_name, + "needs": prerequisite_feature_name, + "segment": { + "id": segment_id, + "name": segment_name, + "condition_json_path": condition_json_path, + }, + } + ) + return edges diff --git a/api/tests/unit/features/dependencies/test_services.py b/api/tests/unit/features/dependencies/test_services.py new file mode 100644 index 000000000000..9aade7a94098 --- /dev/null +++ b/api/tests/unit/features/dependencies/test_services.py @@ -0,0 +1,38 @@ +import pytest +from pytest_django import DjangoAssertNumQueries + +from environments.models import Environment +from features.dependencies.services import validate_segment_flag_dependencies +from features.models import Feature, FeatureSegment, FeatureState +from projects.models import Project +from segments.models import Segment + + +@pytest.mark.parametrize("environment_count", [1, 2, 3]) +def test_validate_segment_flag_dependencies__overrides_across_environments__queries_once_per_environment( + django_assert_num_queries: DjangoAssertNumQueries, + environment_count: int, + project: Project, + segment: Segment, +) -> None: + # Given + for environment_index in range(environment_count): + environment = Environment.objects.create( + name=f"environment{environment_index}", project=project + ) + for feature_name in ["chicken", "egg", "hen"]: + feature, _ = Feature.objects.get_or_create( + name=feature_name, project=project + ) + feature_segment = FeatureSegment.objects.create( + feature=feature, segment=segment, environment=environment + ) + FeatureState.objects.create( + feature=feature, + environment=environment, + feature_segment=feature_segment, + ) + + # When / Then + with django_assert_num_queries(1 + environment_count): + validate_segment_flag_dependencies(segment) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 1d8f8320a65b..e4b3ea0634d4 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -272,7 +272,7 @@ Attributes: ### `features.dependencies.create_failed` Logged at `info` from: - - `api/features/dependencies/services.py:154` + - `api/features/dependencies/services.py:128` Attributes: - `environment.key` From f7b8ac7b01b8953f8e05ef1bfe2d4db68b3b690e Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Fri, 18 Sep 2026 13:10:14 -0300 Subject: [PATCH 20/21] fix test coverage --- .../dependencies/test_segment_management.py | 89 ++----------------- 1 file changed, 5 insertions(+), 84 deletions(-) diff --git a/api/tests/integration/features/dependencies/test_segment_management.py b/api/tests/integration/features/dependencies/test_segment_management.py index af3ae81ca3c3..f6d939968460 100644 --- a/api/tests/integration/features/dependencies/test_segment_management.py +++ b/api/tests/integration/features/dependencies/test_segment_management.py @@ -314,6 +314,7 @@ def test_delete_segment__flag_dependency__indexes_removed( def test_update_segment_add_override__circular_flag_dependency__responds_400( admin_client: APIClient, + create_segment_override: CreateSegmentOverrideFixture, environment: int, environment_api_key: str, environment_name: str, @@ -331,10 +332,10 @@ def test_update_segment_add_override__circular_flag_dependency__responds_400( prerequisite_feature=egg, condition_json_path="$[0].conditions[1]", ) - FeatureSegment.objects.create( - segment=segment1, - feature=chicken, - environment_id=environment, + create_segment_override( + environment_api_key=environment_api_key, + feature_id=chicken.id, + segment_id=segment1.id, ) SegmentFlagReference.objects.create( segment=segment2, @@ -929,83 +930,3 @@ def test_update_segment_update_rules__shared_prerequisite__responds_200( feature__name="chicken", prerequisite_feature__name="egg", ) - - -def test_create_feature_segment__circular_flag_dependency__responds_400( - admin_client: APIClient, - create_segment_override: CreateSegmentOverrideFixture, - environment: int, - environment_api_key: str, - environment_name: str, - log: StructuredLogCapture, - organisation: int, - project: int, -) -> None: - # Given - chicken = Feature.objects.create(name="chicken", project_id=project) - egg = Feature.objects.create(name="egg", project_id=project) - chicken_segment = Segment.objects.create(name="chicken_segment", project_id=project) - egg_segment = Segment.objects.create(name="egg_segment", project_id=project) - SegmentFlagReference.objects.create( - segment=chicken_segment, - prerequisite_feature=egg, - condition_json_path="$[0].conditions[1]", - ) - create_segment_override( - environment_api_key=environment_api_key, - feature_id=chicken.id, - segment_id=chicken_segment.id, - ) - SegmentFlagReference.objects.create( - segment=egg_segment, - prerequisite_feature=chicken, - condition_json_path="$[1].conditions[2]", - ) - - # When - response = admin_client.post( - "/api/v1/features/feature-segments/", - data={ - "feature": egg.id, - "segment": egg_segment.id, - "environment": environment, - }, - format="json", - ) - - # Then - assert response.status_code == 400 - assert response.json() == { - "code": "circular_dependency", - "environment": {"key": environment_api_key, "name": environment_name}, - "path": [ - { - "feature": "egg", - "needs": "chicken", - "segment": { - "id": egg_segment.id, - "name": egg_segment.name, - "condition_json_path": "$[1].conditions[2]", - }, - }, - { - "feature": "chicken", - "needs": "egg", - "segment": { - "id": chicken_segment.id, - "name": chicken_segment.name, - "condition_json_path": "$[0].conditions[1]", - }, - }, - ], - } - assert not FeatureSegment.objects.filter(segment=egg_segment, feature=egg).exists() - assert log.has( - "dependencies.create_failed", - level="info", - organisation__id=organisation, - project__id=project, - environment__key=environment_api_key, - feature__name="egg", - prerequisite_feature__name="chicken", - ) From 22199e1b8499d3f775f75f81535cb47d21ad1cca Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Fri, 18 Sep 2026 13:58:54 -0300 Subject: [PATCH 21/21] short-circuit --- api/features/dependencies/services.py | 3 +++ .../features/dependencies/test_services.py | 27 ++++++++++++++++++- .../observability/_events-catalogue.md | 2 +- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index 619dad28625e..c9a0dfddf4a3 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -105,6 +105,9 @@ def report_flag_dependencies( def validate_segment_flag_dependencies(segment: "Segment") -> None: """Raise if any feature the segment overrides ends up depending on itself.""" + existing_references = SegmentFlagReference.objects.filter(segment=segment) + if not existing_references.exists(): + return edges_by_environment_id: dict[int, dict[FeatureName, list[DependencyEdge]]] = {} for override in ( get_all_live_or_scheduled_overrides() diff --git a/api/tests/unit/features/dependencies/test_services.py b/api/tests/unit/features/dependencies/test_services.py index 9aade7a94098..6ccdc613bbb8 100644 --- a/api/tests/unit/features/dependencies/test_services.py +++ b/api/tests/unit/features/dependencies/test_services.py @@ -2,6 +2,7 @@ from pytest_django import DjangoAssertNumQueries from environments.models import Environment +from features.dependencies.models import SegmentFlagReference from features.dependencies.services import validate_segment_flag_dependencies from features.models import Feature, FeatureSegment, FeatureState from projects.models import Project @@ -32,7 +33,31 @@ def test_validate_segment_flag_dependencies__overrides_across_environments__quer environment=environment, feature_segment=feature_segment, ) + SegmentFlagReference.objects.create( + segment=segment, + prerequisite_feature=Feature.objects.create(name="corn", project=project), + condition_json_path="$[0].conditions[0]", + ) # When / Then - with django_assert_num_queries(1 + environment_count): + with django_assert_num_queries(2 + environment_count): + validate_segment_flag_dependencies(segment) + + +def test_validate_segment_flag_dependencies__no_references__skips( + django_assert_num_queries: DjangoAssertNumQueries, + environment: Environment, + feature: Feature, + segment: Segment, +) -> None: + # Given + feature_segment = FeatureSegment.objects.create( + feature=feature, segment=segment, environment=environment + ) + FeatureState.objects.create( + feature=feature, environment=environment, feature_segment=feature_segment + ) + + # When / Then + with django_assert_num_queries(1): validate_segment_flag_dependencies(segment) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 95e0fbcfe8b2..79394941051d 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -272,7 +272,7 @@ Attributes: ### `features.dependencies.create_failed` Logged at `info` from: - - `api/features/dependencies/services.py:128` + - `api/features/dependencies/services.py:131` Attributes: - `environment.key`