Skip to content

Commit 28821fa

Browse files
authored
fix(cache_page): use vary on header with env api key (#5800)
1 parent 0b6ec10 commit 28821fa

File tree

6 files changed

+126
-5
lines changed

6 files changed

+126
-5
lines changed

api/core/constants.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@
88
FLAGSMITH_SIGNATURE_HEADER = "X-Flagsmith-Signature"
99

1010
FLAGSMITH_UPDATED_AT_HEADER = "X-Flagsmith-Document-Updated-At"
11+
SDK_ENVIRONMENT_KEY_HEADER = "X_ENVIRONMENT_KEY"

api/environments/identities/views.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@
1010
from django.utils import timezone
1111
from django.utils.decorators import method_decorator
1212
from django.views.decorators.cache import cache_page
13+
from django.views.decorators.vary import vary_on_headers
1314
from drf_yasg.utils import swagger_auto_schema # type: ignore[import-untyped]
1415
from rest_framework import status, viewsets
1516
from rest_framework.permissions import IsAuthenticated
1617
from rest_framework.response import Response
1718

1819
from app.pagination import CustomPagination
19-
from core.constants import FLAGSMITH_UPDATED_AT_HEADER
20+
from core.constants import FLAGSMITH_UPDATED_AT_HEADER, SDK_ENVIRONMENT_KEY_HEADER
2021
from core.request_origin import RequestOrigin
2122
from edge_api.identities.tasks import forward_identity_request
2223
from environments.identities.models import Identity
@@ -161,6 +162,7 @@ class SDKIdentities(SDKAPIView):
161162
query_serializer=SDKIdentitiesQuerySerializer(),
162163
operation_id="identify_user",
163164
)
165+
@method_decorator(vary_on_headers(SDK_ENVIRONMENT_KEY_HEADER))
164166
@method_decorator(
165167
cache_page(
166168
timeout=settings.GET_IDENTITIES_ENDPOINT_CACHE_SECONDS,

api/features/views.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from django.utils import timezone
1111
from django.utils.decorators import method_decorator
1212
from django.views.decorators.cache import cache_page
13+
from django.views.decorators.vary import vary_on_headers
1314
from drf_yasg import openapi # type: ignore[import-untyped]
1415
from drf_yasg.utils import swagger_auto_schema # type: ignore[import-untyped]
1516
from rest_framework import mixins, serializers, status, viewsets
@@ -24,7 +25,7 @@
2425
from app.pagination import CustomPagination
2526
from app_analytics.analytics_db_service import get_feature_evaluation_data
2627
from app_analytics.influxdb_wrapper import get_multiple_event_list_for_feature
27-
from core.constants import FLAGSMITH_UPDATED_AT_HEADER
28+
from core.constants import FLAGSMITH_UPDATED_AT_HEADER, SDK_ENVIRONMENT_KEY_HEADER
2829
from core.request_origin import RequestOrigin
2930
from environments.authentication import EnvironmentKeyAuthentication
3031
from environments.identities.models import Identity
@@ -782,6 +783,7 @@ class SDKFeatureStates(GenericAPIView): # type: ignore[type-arg]
782783
query_serializer=SDKFeatureStatesQuerySerializer(),
783784
responses={200: FeatureStateSerializerFull(many=True)},
784785
)
786+
@method_decorator(vary_on_headers(SDK_ENVIRONMENT_KEY_HEADER))
785787
@method_decorator(
786788
cache_page(
787789
timeout=settings.GET_FLAGS_ENDPOINT_CACHE_SECONDS,

api/tests/unit/conftest.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
from unittest.mock import MagicMock
22

33
import pytest
4-
from django.core.cache import BaseCache
4+
from django.core.cache import BaseCache, caches
5+
from django.core.cache.backends.locmem import LocMemCache
56
from pytest_django.fixtures import SettingsWrapper
67
from pytest_mock import MockerFixture
78

@@ -232,3 +233,23 @@ def populate_environment_document_cache(
232233
persistent_environment_document_cache.get.return_value = (
233234
map_environment_to_environment_document(environment)
234235
)
236+
237+
238+
@pytest.fixture()
239+
def use_local_mem_cache_for_cache_middleware(mocker: MockerFixture) -> None:
240+
# Ensure the default cache is LocMemCache
241+
default_cache = caches["default"]
242+
assert isinstance(default_cache, LocMemCache)
243+
244+
# Patch CacheMiddleware to use 'default' cache and a non-zero timeout
245+
# This is necessary because override_settings doesn't reliably affect middleware behavior
246+
from django.middleware.cache import CacheMiddleware
247+
248+
original_init = CacheMiddleware.__init__
249+
250+
def custom_init(self, *args, **kwargs) -> None: # type: ignore[no-untyped-def]
251+
original_init(self, *args, **kwargs)
252+
self.page_timeout = 10 # enable caching for the view
253+
self.cache_alias = "default" # force use of in-memory test cache
254+
255+
mocker.patch.object(CacheMiddleware, "__init__", custom_init)

api/tests/unit/environments/identities/test_unit_identities_views.py

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717
from rest_framework.permissions import IsAuthenticated
1818
from rest_framework.test import APIClient
1919

20-
from core.constants import FLAGSMITH_UPDATED_AT_HEADER, STRING
20+
from core.constants import (
21+
FLAGSMITH_UPDATED_AT_HEADER,
22+
SDK_ENVIRONMENT_KEY_HEADER,
23+
STRING,
24+
)
2125
from environments.identities.helpers import (
2226
get_hashed_percentage_for_object_ids,
2327
)
@@ -338,6 +342,54 @@ def test_identities_endpoint_returns_all_feature_states_for_identity_if_feature_
338342
assert len(response.data["flags"]) == 2
339343

340344

345+
def test_get_flags_for_identities_with_cache(
346+
environment: Environment,
347+
feature: Feature,
348+
django_assert_num_queries: DjangoAssertNumQueries,
349+
use_local_mem_cache_for_cache_middleware: None,
350+
project_two_feature: Feature,
351+
project_two_environment: Environment,
352+
) -> None:
353+
# Given
354+
base_url = reverse("api-v1:sdk-identities")
355+
url = base_url + "?identifier=some-identifier"
356+
357+
# Create clients for two separate environments
358+
environment_one_client = APIClient(
359+
headers={SDK_ENVIRONMENT_KEY_HEADER: environment.api_key}
360+
)
361+
project_two_environment_client = APIClient(
362+
headers={SDK_ENVIRONMENT_KEY_HEADER: project_two_environment.api_key}
363+
)
364+
365+
# Fetch flags for both environments once to warm the cache
366+
environment_one_response = environment_one_client.get(url)
367+
assert environment_one_response.status_code == status.HTTP_200_OK
368+
369+
project_two_environment_response = project_two_environment_client.get(url)
370+
assert project_two_environment_response.status_code == status.HTTP_200_OK
371+
372+
# When
373+
with django_assert_num_queries(0):
374+
for _ in range(10):
375+
environment_one_response = environment_one_client.get(url)
376+
assert environment_one_response.status_code == status.HTTP_200_OK
377+
378+
project_two_environment_response = project_two_environment_client.get(url)
379+
assert project_two_environment_response.status_code == status.HTTP_200_OK
380+
381+
# Then
382+
# Each response must return the correct feature for its environment
383+
assert (
384+
environment_one_response.json()["flags"][0]["feature"]["id"]
385+
== feature.id
386+
)
387+
assert (
388+
project_two_environment_response.json()["flags"][0]["feature"]["id"]
389+
== project_two_feature.id
390+
)
391+
392+
341393
@mock.patch("integrations.amplitude.amplitude.AmplitudeWrapper.identify_user_async")
342394
def test_identities_endpoint_get_all_feature_amplitude_called(
343395
mock_amplitude_wrapper: mock.MagicMock,

api/tests/unit/features/test_unit_features_views.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
IDENTITY_FEATURE_STATE_UPDATED_MESSAGE,
3535
)
3636
from audit.models import AuditLog, RelatedObjectType # type: ignore[attr-defined]
37-
from core.constants import FLAGSMITH_UPDATED_AT_HEADER
37+
from core.constants import FLAGSMITH_UPDATED_AT_HEADER, SDK_ENVIRONMENT_KEY_HEADER
3838
from environments.dynamodb import (
3939
DynamoEnvironmentV2Wrapper,
4040
DynamoIdentityWrapper,
@@ -799,6 +799,49 @@ def test_get_flags__server_key_only_feature__return_expected(
799799
assert not response.json()
800800

801801

802+
def test_get_flags_cache(
803+
environment: Environment,
804+
feature: Feature,
805+
django_assert_num_queries: DjangoAssertNumQueries,
806+
project_two_feature: Feature,
807+
project_two_environment: Environment,
808+
use_local_mem_cache_for_cache_middleware: None,
809+
) -> None:
810+
# Given
811+
url = reverse("api-v1:flags")
812+
813+
# Create clients for two separate environments
814+
environment_one_client = APIClient(
815+
headers={SDK_ENVIRONMENT_KEY_HEADER: environment.api_key}
816+
)
817+
project_two_environment_client = APIClient(
818+
headers={SDK_ENVIRONMENT_KEY_HEADER: project_two_environment.api_key}
819+
)
820+
# Fetch flags for both environments once to warm the cache
821+
environment_one_response = environment_one_client.get(url)
822+
assert environment_one_response.status_code == status.HTTP_200_OK
823+
824+
project_two_environment_response = project_two_environment_client.get(url)
825+
assert project_two_environment_response.status_code == status.HTTP_200_OK
826+
827+
# When
828+
with django_assert_num_queries(0):
829+
for _ in range(10):
830+
environment_one_response = environment_one_client.get(url)
831+
assert environment_one_response.status_code == status.HTTP_200_OK
832+
833+
project_two_environment_response = project_two_environment_client.get(url)
834+
assert project_two_environment_response.status_code == status.HTTP_200_OK
835+
836+
# Then
837+
# Each response must return the correct feature for its environment
838+
assert environment_one_response.json()[0]["feature"]["id"] == feature.id
839+
assert (
840+
project_two_environment_response.json()[0]["feature"]["id"]
841+
== project_two_feature.id
842+
)
843+
844+
802845
def test_get_flags__server_key_only_feature__server_key_auth__return_expected(
803846
api_client: APIClient,
804847
environment_api_key: EnvironmentAPIKey,

0 commit comments

Comments
 (0)