From 9a677ee03da9bf70688355c2b1b0463d31f34dbd Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Fri, 6 Mar 2026 17:58:39 +0000 Subject: [PATCH 1/4] fix: `/api/v1/environment-document` N+1 query issue related to the number of segment overrides in a version (#6865) --- api/environments/managers.py | 5 +- ...unit_environments_views_sdk_environment.py | 207 +++++++++++++++++- 2 files changed, 204 insertions(+), 8 deletions(-) diff --git a/api/environments/managers.py b/api/environments/managers.py index fe0e0a4b51df..d89c1bf8cc7e 100644 --- a/api/environments/managers.py +++ b/api/environments/managers.py @@ -34,7 +34,10 @@ def filter_for_document_builder( Prefetch( "project__segments__feature_segments__feature_states", queryset=FeatureState.objects.select_related( - "feature", "feature_state_value", "environment" + "feature", + "feature_state_value", + "environment", + "environment_feature_version", ), ), Prefetch( diff --git a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py index c14b976d5bcd..87b2859bef0b 100644 --- a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py +++ b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py @@ -1,15 +1,21 @@ +import time from typing import TYPE_CHECKING +from unittest.mock import ANY +import pytest from core.constants import FLAGSMITH_UPDATED_AT_HEADER from django.urls import reverse +from django.utils import timezone +from django.utils.http import http_date from flag_engine.segments.constants import EQUAL +from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped] from rest_framework import status from rest_framework.test import APIClient from environments.identities.models import Identity from environments.models import Environment, EnvironmentAPIKey from features.feature_types import MULTIVARIATE -from features.models import ( +from features.models import ( # type: ignore[attr-defined] STRING, Feature, FeatureSegment, @@ -17,25 +23,36 @@ FeatureStateValue, ) from features.multivariate.models import MultivariateFeatureOption +from features.versioning.models import EnvironmentFeatureVersion +from features.versioning.tasks import enable_v2_versioning +from projects.models import Project from segments.models import Condition, Segment, SegmentRule if TYPE_CHECKING: from pytest_django import DjangoAssertNumQueries from organisations.models import Organisation - from projects.models import Project +@pytest.mark.parametrize( + "use_v2_feature_versioning, total_queries", [(True, 12), (False, 11)] +) def test_get_environment_document( organisation_one: "Organisation", + organisation_two: "Organisation", organisation_one_project_one: "Project", django_assert_num_queries: "DjangoAssertNumQueries", + use_v2_feature_versioning: bool, + total_queries: int, ) -> None: # Given project = organisation_one_project_one - # an environment - environment = Environment.objects.create(name="Test Environment", project=project) + environment = Environment.objects.create( + name="Test Environment", + project=project, + ) + api_key = EnvironmentAPIKey.objects.create(environment=environment) client = APIClient() client.credentials(HTTP_X_ENVIRONMENT_KEY=api_key.key) @@ -63,6 +80,9 @@ def test_get_environment_document( rule=nested_rule, ) + # Segment revisions should not be included in the document + segment.clone(is_revision=True, name=f"revision_segment_{i}") + # Let's create segment override for each segment too feature_segment = FeatureSegment.objects.create( segment=segment, feature=feature, environment=environment @@ -89,7 +109,7 @@ def test_get_environment_document( type=STRING, ) - for i in range(10): + # Add a multivariate feature mv_feature = Feature.objects.create( name=f"mv_feature_{i}", project=project, type=MULTIVARIATE ) @@ -104,21 +124,135 @@ def test_get_environment_document( string_value="option-2", ) + if use_v2_feature_versioning: + enable_v2_versioning(environment.id) + + # create new versions of a given featurestate + for _ in range(2): + efv = EnvironmentFeatureVersion.objects.create( + environment=environment, + feature=feature, + ) + efv.publish() + # and the relevant URL to get an environment document url = reverse("api-v1:environment-document") # When - with django_assert_num_queries(15): + with django_assert_num_queries(total_queries): response = client.get(url) # Then assert response.status_code == status.HTTP_200_OK - assert response.json() + assert len(response.data["project"]["segments"]) == 10 + assert len(response.data["feature_states"]) == 11 + assert len(response.data["identity_overrides"]) == 10 + + environment.refresh_from_db() assert response.headers[FLAGSMITH_UPDATED_AT_HEADER] == str( environment.updated_at.timestamp() ) +@pytest.mark.parametrize( + "environment_", + ( + lazy_fixture("environment"), + lazy_fixture("environment_v2_versioning"), + ), +) +def test_get_environment_document__identity_overrides( + project: Project, + environment_: Environment, +) -> None: + # Given + api_key = EnvironmentAPIKey.objects.create(environment=environment_) + client = APIClient() + client.credentials(HTTP_X_ENVIRONMENT_KEY=api_key.key) + + feature = Feature.objects.create(name="test_feature", project=project) + for i in range(2): + identity = Identity.objects.create( + environment=environment_, + identifier=f"identity_{i}", + ) + identity_feature_state = FeatureState.objects.create( + identity=identity, + feature=feature, + environment=environment_, + ) + FeatureStateValue.objects.filter(feature_state=identity_feature_state).update( + string_value="overridden", + type=STRING, + ) + + url = reverse("api-v1:environment-document") + + # When + response = client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert sorted( + response.data["identity_overrides"], + key=lambda x: x["identifier"], + ) == [ + { + "composite_key": ANY, + "created_date": ANY, + "django_id": ANY, + "environment_api_key": environment_.api_key, + "identifier": "identity_0", + "dashboard_alias": None, + "identity_features": [ + { + "django_id": ANY, + "enabled": False, + "feature": { + "id": feature.id, + "name": "test_feature", + "type": "STANDARD", + }, + "feature_segment": None, + "feature_state_value": "overridden", + "featurestate_uuid": ANY, + "multivariate_feature_state_values": [], + } + ], + "identity_traits": [], + "identity_uuid": ANY, + }, + { + "composite_key": ANY, + "created_date": ANY, + "django_id": ANY, + "environment_api_key": environment_.api_key, + "identifier": "identity_1", + "dashboard_alias": None, + "identity_features": [ + { + "django_id": ANY, + "enabled": False, + "feature": { + "id": feature.id, + "name": "test_feature", + "type": "STANDARD", + }, + "feature_segment": None, + "feature_state_value": "overridden", + "featurestate_uuid": ANY, + "multivariate_feature_state_values": [], + } + ], + "identity_traits": [], + "identity_uuid": ANY, + }, + ] + assert response.headers[FLAGSMITH_UPDATED_AT_HEADER] == str( + environment_.updated_at.timestamp() + ) + + def test_get_environment_document_fails_with_invalid_key( organisation_one: "Organisation", organisation_one_project_one: "Project", @@ -143,3 +277,62 @@ def test_get_environment_document_fails_with_invalid_key( # We get a 403 since only the server side API keys are able to access the # environment document assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_environment_document_if_modified_since( + organisation_one: "Organisation", + organisation_one_project_one: "Project", +) -> None: + # Given + project = organisation_one_project_one + environment = Environment.objects.create(name="Test Environment", project=project) + api_key = EnvironmentAPIKey.objects.create(environment=environment).key + + client = APIClient() + client.credentials(HTTP_X_ENVIRONMENT_KEY=api_key) + url = reverse("api-v1:environment-document") + + # When - first request + response1 = client.get(url) + + # Then - first request should return 200 and include Last-Modified header + assert response1.status_code == status.HTTP_200_OK + last_modified = response1.headers["Last-Modified"] + assert last_modified == http_date(environment.updated_at.timestamp()) + + # When - second request with If-Modified-Since header + client.credentials( + HTTP_X_ENVIRONMENT_KEY=api_key, + HTTP_IF_MODIFIED_SINCE=last_modified, + ) + response2 = client.get(url) + + # Then - second request should return 304 Not Modified + assert response2.status_code == status.HTTP_304_NOT_MODIFIED + assert len(response2.content) == 0 + + # sleep for 1s since If-Modified-Since is only accurate to the nearest second + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/If-Modified-Since + time.sleep(1) + + # When - environment is updated + environment.updated_at = timezone.now() + environment.save() + + # Then - request with same If-Modified-Since header should return 200 + response3 = client.get(url) + assert response3.status_code == status.HTTP_200_OK + assert response3.headers["Last-Modified"] == http_date( + environment.updated_at.timestamp() + ) + + # When - request without If-Modified-Since header + client.credentials( + HTTP_X_ENVIRONMENT_KEY=api_key, + HTTP_IF_MODIFIED_SINCE="", + ) + response4 = client.get(url) + + # Then - actual environment is returned with a 200 + assert response4.status_code == status.HTTP_200_OK + assert len(response4.content) > 0 From a0311cacfcdfc2e1f03bc13dd361ebae1c488574 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Mon, 9 Mar 2026 16:13:57 +0000 Subject: [PATCH 2/4] Remove new tests --- ...unit_environments_views_sdk_environment.py | 163 ------------------ 1 file changed, 163 deletions(-) diff --git a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py index 87b2859bef0b..ab57520ed953 100644 --- a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py +++ b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py @@ -1,14 +1,9 @@ -import time from typing import TYPE_CHECKING -from unittest.mock import ANY import pytest from core.constants import FLAGSMITH_UPDATED_AT_HEADER from django.urls import reverse -from django.utils import timezone -from django.utils.http import http_date from flag_engine.segments.constants import EQUAL -from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped] from rest_framework import status from rest_framework.test import APIClient @@ -154,105 +149,6 @@ def test_get_environment_document( ) -@pytest.mark.parametrize( - "environment_", - ( - lazy_fixture("environment"), - lazy_fixture("environment_v2_versioning"), - ), -) -def test_get_environment_document__identity_overrides( - project: Project, - environment_: Environment, -) -> None: - # Given - api_key = EnvironmentAPIKey.objects.create(environment=environment_) - client = APIClient() - client.credentials(HTTP_X_ENVIRONMENT_KEY=api_key.key) - - feature = Feature.objects.create(name="test_feature", project=project) - for i in range(2): - identity = Identity.objects.create( - environment=environment_, - identifier=f"identity_{i}", - ) - identity_feature_state = FeatureState.objects.create( - identity=identity, - feature=feature, - environment=environment_, - ) - FeatureStateValue.objects.filter(feature_state=identity_feature_state).update( - string_value="overridden", - type=STRING, - ) - - url = reverse("api-v1:environment-document") - - # When - response = client.get(url) - - # Then - assert response.status_code == status.HTTP_200_OK - assert sorted( - response.data["identity_overrides"], - key=lambda x: x["identifier"], - ) == [ - { - "composite_key": ANY, - "created_date": ANY, - "django_id": ANY, - "environment_api_key": environment_.api_key, - "identifier": "identity_0", - "dashboard_alias": None, - "identity_features": [ - { - "django_id": ANY, - "enabled": False, - "feature": { - "id": feature.id, - "name": "test_feature", - "type": "STANDARD", - }, - "feature_segment": None, - "feature_state_value": "overridden", - "featurestate_uuid": ANY, - "multivariate_feature_state_values": [], - } - ], - "identity_traits": [], - "identity_uuid": ANY, - }, - { - "composite_key": ANY, - "created_date": ANY, - "django_id": ANY, - "environment_api_key": environment_.api_key, - "identifier": "identity_1", - "dashboard_alias": None, - "identity_features": [ - { - "django_id": ANY, - "enabled": False, - "feature": { - "id": feature.id, - "name": "test_feature", - "type": "STANDARD", - }, - "feature_segment": None, - "feature_state_value": "overridden", - "featurestate_uuid": ANY, - "multivariate_feature_state_values": [], - } - ], - "identity_traits": [], - "identity_uuid": ANY, - }, - ] - assert response.headers[FLAGSMITH_UPDATED_AT_HEADER] == str( - environment_.updated_at.timestamp() - ) - - def test_get_environment_document_fails_with_invalid_key( organisation_one: "Organisation", organisation_one_project_one: "Project", @@ -277,62 +173,3 @@ def test_get_environment_document_fails_with_invalid_key( # We get a 403 since only the server side API keys are able to access the # environment document assert response.status_code == status.HTTP_403_FORBIDDEN - - -def test_environment_document_if_modified_since( - organisation_one: "Organisation", - organisation_one_project_one: "Project", -) -> None: - # Given - project = organisation_one_project_one - environment = Environment.objects.create(name="Test Environment", project=project) - api_key = EnvironmentAPIKey.objects.create(environment=environment).key - - client = APIClient() - client.credentials(HTTP_X_ENVIRONMENT_KEY=api_key) - url = reverse("api-v1:environment-document") - - # When - first request - response1 = client.get(url) - - # Then - first request should return 200 and include Last-Modified header - assert response1.status_code == status.HTTP_200_OK - last_modified = response1.headers["Last-Modified"] - assert last_modified == http_date(environment.updated_at.timestamp()) - - # When - second request with If-Modified-Since header - client.credentials( - HTTP_X_ENVIRONMENT_KEY=api_key, - HTTP_IF_MODIFIED_SINCE=last_modified, - ) - response2 = client.get(url) - - # Then - second request should return 304 Not Modified - assert response2.status_code == status.HTTP_304_NOT_MODIFIED - assert len(response2.content) == 0 - - # sleep for 1s since If-Modified-Since is only accurate to the nearest second - # https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/If-Modified-Since - time.sleep(1) - - # When - environment is updated - environment.updated_at = timezone.now() - environment.save() - - # Then - request with same If-Modified-Since header should return 200 - response3 = client.get(url) - assert response3.status_code == status.HTTP_200_OK - assert response3.headers["Last-Modified"] == http_date( - environment.updated_at.timestamp() - ) - - # When - request without If-Modified-Since header - client.credentials( - HTTP_X_ENVIRONMENT_KEY=api_key, - HTTP_IF_MODIFIED_SINCE="", - ) - response4 = client.get(url) - - # Then - actual environment is returned with a 200 - assert response4.status_code == status.HTTP_200_OK - assert len(response4.content) > 0 From 40aa459b443fafeeaeae44040fbe6b191d4cd591 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Mon, 9 Mar 2026 16:34:02 +0000 Subject: [PATCH 3/4] Remove segment cloning --- .../test_unit_environments_views_sdk_environment.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py index ab57520ed953..92d8ec8fa899 100644 --- a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py +++ b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py @@ -75,9 +75,6 @@ def test_get_environment_document( rule=nested_rule, ) - # Segment revisions should not be included in the document - segment.clone(is_revision=True, name=f"revision_segment_{i}") - # Let's create segment override for each segment too feature_segment = FeatureSegment.objects.create( segment=segment, feature=feature, environment=environment From 21b91022378d655855a1eb31150deee926e5d188 Mon Sep 17 00:00:00 2001 From: Gagan Date: Fri, 11 Apr 2025 13:42:30 +0530 Subject: [PATCH 4/4] fix(n+1): fix environment-document n+1 for env_feature_version (#5332) --- api/environments/models.py | 1 + .../test_unit_environments_views_sdk_environment.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api/environments/models.py b/api/environments/models.py index f85b6851d877..7d67a8e08a38 100644 --- a/api/environments/models.py +++ b/api/environments/models.py @@ -406,6 +406,7 @@ def _get_environment_document_from_db( "feature", "feature_state_value", "identity", + "environment_feature_version", "identity__environment", ).prefetch_related( Prefetch( diff --git a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py index 92d8ec8fa899..f62c89ba7603 100644 --- a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py +++ b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py @@ -30,7 +30,7 @@ @pytest.mark.parametrize( - "use_v2_feature_versioning, total_queries", [(True, 12), (False, 11)] + "use_v2_feature_versioning, total_queries", [(True, 16), (False, 15)] ) def test_get_environment_document( organisation_one: "Organisation",