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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion api/environments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ def create_feature_states(self) -> None:
@hook(AFTER_UPDATE) # type: ignore[misc]
def clear_environment_cache(self) -> None:
# TODO: this could rebuild the cache itself (using an async task)
environment_cache.delete(self.initial_value("api_key"))
environment_cache.delete_many(
[self.initial_value("api_key"), *[eak.key for eak in self.api_keys.all()]]
)

@hook(AFTER_UPDATE, when="api_key", has_changed=True) # type: ignore[misc]
def update_environment_document_cache(self) -> None:
Expand Down
21 changes: 17 additions & 4 deletions api/environments/sdk/views.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,27 @@
from django.http import HttpRequest
from datetime import datetime
from typing import Optional

from django.utils.decorators import method_decorator
from django.views.decorators.http import condition
from drf_yasg.utils import swagger_auto_schema # type: ignore[import-untyped]
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.views import APIView

from core.constants import FLAGSMITH_UPDATED_AT_HEADER
from environments.authentication import EnvironmentKeyAuthentication
from environments.authentication import (
EnvironmentKeyAuthentication,
)
from environments.models import Environment
from environments.permissions.permissions import EnvironmentKeyPermissions
from environments.sdk.schemas import SDKEnvironmentDocumentModel


def get_last_modified(request: Request) -> datetime | None:
updated_at: Optional[datetime] = request.environment.updated_at
return updated_at


class SDKEnvironmentAPIView(APIView):
permission_classes = (EnvironmentKeyPermissions,)
throttle_classes = []
Expand All @@ -18,9 +30,10 @@ def get_authenticators(self): # type: ignore[no-untyped-def]
return [EnvironmentKeyAuthentication(required_key_prefix="ser.")]

@swagger_auto_schema(responses={200: SDKEnvironmentDocumentModel}) # type: ignore[misc]
def get(self, request: HttpRequest) -> Response:
@method_decorator(condition(last_modified_func=get_last_modified))
def get(self, request: Request) -> Response:
environment_document = Environment.get_environment_document(
request.environment.api_key # type: ignore[attr-defined]
request.environment.api_key,
)
updated_at = self.request.environment.updated_at
return Response(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,9 @@ def test_save_environment_clears_environment_cache(mocker, project): # type: ig
environment.save()

# Then
mock_calls = mock_environment_cache.delete.mock_calls
mock_calls = mock_environment_cache.delete_many.mock_calls
assert len(mock_calls) == 2
assert mock_calls[0][1][0] == mock_calls[1][1][0] == old_key
assert mock_calls[0][1][0] == mock_calls[1][1][0] == [old_key]


@pytest.mark.parametrize(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import time
from typing import TYPE_CHECKING

import pytest
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 rest_framework import status
from rest_framework.test import APIClient
Expand Down Expand Up @@ -175,3 +178,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
Comment on lines +211 to +212
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also make a request from a client that doesn't have the If-Modified-Since header to make sure that it still returns the environment document?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 957e6ff

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
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,4 @@ def test_amplitude_configuration_update_clears_environment_cache(environment, mo
amplitude_config.save()

# Then
mock_environment_cache.delete.assert_called_once_with(environment.api_key)
mock_environment_cache.delete_many.assert_called_once_with([environment.api_key])
Loading