diff --git a/api/conftest.py b/api/conftest.py index 6f3600650e4c..7e32e881dfe4 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -2,6 +2,7 @@ import os import typing from unittest.mock import MagicMock +from uuid import UUID import boto3 import pytest @@ -394,6 +395,16 @@ def environment(project): return Environment.objects.create(name="Test Environment", project=project) +@pytest.fixture() +def environment_uuid(environment: Environment) -> UUID: + return environment.uuid + + +@pytest.fixture() +def environment_client_api_key(environment: Environment) -> str: + return environment.api_key + + @pytest.fixture() def with_environment_permissions( environment: Environment, staff_user: FFAdminUser diff --git a/api/edge_api/identities/export.py b/api/edge_api/identities/export.py index ebfebb85f498..517ea59f9670 100644 --- a/api/edge_api/identities/export.py +++ b/api/edge_api/identities/export.py @@ -8,6 +8,7 @@ from edge_api.identities.models import EdgeIdentity from environments.identities.traits.models import Trait +from environments.models import Environment from features.models import Feature, FeatureState from features.multivariate.models import MultivariateFeatureOption @@ -27,19 +28,19 @@ def export_edge_identity_and_overrides( # noqa: C901 traits_export = [] identity_override_export = [] + environment_uuid = Environment.objects.get(api_key=environment_api_key).uuid feature_id_to_uuid: dict[int, str] = get_feature_uuid_cache(environment_api_key) mv_feature_option_id_to_uuid: dict[int, str] = get_mv_feature_option_uuid_cache( environment_api_key ) + while True: response = EdgeIdentity.dynamo_wrapper.get_all_items(**kwargs) for item in response["Items"]: identifier = item["identifier"] # export identity identity_export.append( - export_edge_identity( - identifier, environment_api_key, item["created_date"] - ) + export_edge_identity(identifier, environment_uuid, item["created_date"]) ) # export traits for trait in item["identity_traits"]: @@ -60,6 +61,7 @@ def export_edge_identity_and_overrides( # noqa: C901 export_edge_feature_state( identifier, environment_api_key, + environment_uuid, featurestate_uuid, feature_uuid, override["enabled"], @@ -132,14 +134,14 @@ def export_edge_trait(trait: dict, identifier: str, environment_api_key: str) -> def export_edge_identity( - identifier: str, environment_api_key: str, created_date: str + identifier: str, environment_uuid: str, created_date: str ) -> dict: return { "model": "identities.identity", "fields": { "identifier": identifier, "created_date": created_date, - "environment": [environment_api_key], + "environment": [environment_uuid], }, } @@ -147,6 +149,7 @@ def export_edge_identity( def export_edge_feature_state( identifier: str, environment_api_key: str, + environment_uuid: str, featurestate_uuid: str, feature_uuid: str, enabled: bool, @@ -162,7 +165,7 @@ def export_edge_feature_state( "updated_at": timezone.now().isoformat(), "live_from": timezone.now().isoformat(), "feature": [feature_uuid], - "environment": [environment_api_key], + "environment": [environment_uuid], "identity": [ identifier, environment_api_key, diff --git a/api/environments/managers.py b/api/environments/managers.py index c170bc44ea28..a9bae3a32e3e 100644 --- a/api/environments/managers.py +++ b/api/environments/managers.py @@ -1,3 +1,5 @@ +import typing + from django.db.models import Prefetch from softdelete.models import SoftDeleteManager @@ -5,6 +7,9 @@ from features.multivariate.models import MultivariateFeatureStateValue from segments.models import Segment +if typing.TYPE_CHECKING: + from environments.models import Environment + class EnvironmentManager(SoftDeleteManager): def filter_for_document_builder( @@ -55,5 +60,5 @@ def filter_for_document_builder( def get_queryset(self): return super().get_queryset().select_related("project", "project__organisation") - def get_by_natural_key(self, api_key): - return self.get(api_key=api_key) + def get_by_natural_key(self, uuid: str) -> "Environment": + return self.get(uuid=uuid) diff --git a/api/environments/migrations/0037_add_uuid_field.py b/api/environments/migrations/0037_add_uuid_field.py new file mode 100644 index 000000000000..7177133bd7b8 --- /dev/null +++ b/api/environments/migrations/0037_add_uuid_field.py @@ -0,0 +1,30 @@ +# Generated by Django 3.2.23 on 2024-11-27 08:46 + +from django.db import migrations, models +import uuid + +from core.migration_helpers import AddDefaultUUIDs + + +class Migration(migrations.Migration): + atomic = False # avoid long running transaction + dependencies = [ + ("environments", "0036_add_is_creating_field"), + ] + + operations = [ + migrations.AddField( + model_name="environment", + name="uuid", + field=models.UUIDField(editable=False, null=True), + ), + migrations.RunPython( + AddDefaultUUIDs("environments", "environment"), + reverse_code=migrations.RunPython.noop, + ), + migrations.AlterField( + model_name="environment", + name="uuid", + field=models.UUIDField(default=uuid.uuid4, editable=False, unique=True), + ), + ] diff --git a/api/environments/models.py b/api/environments/models.py index f1cf5adc5132..1117c331c83e 100644 --- a/api/environments/models.py +++ b/api/environments/models.py @@ -1,5 +1,6 @@ import logging import typing +import uuid from copy import deepcopy from core.models import abstract_base_auditable_model_factory @@ -63,13 +64,16 @@ class Environment( LifecycleModel, abstract_base_auditable_model_factory( - change_details_excluded_fields=["updated_at"] + historical_records_excluded_fields=["uuid"], + change_details_excluded_fields=["updated_at"], ), SoftDeleteObject, ): history_record_class_path = "environments.models.HistoricalEnvironment" related_object_type = RelatedObjectType.ENVIRONMENT + uuid = models.UUIDField(default=uuid.uuid4, unique=True, editable=False) + name = models.CharField(max_length=2000) created_date = models.DateTimeField("DateCreated", auto_now_add=True) description = models.TextField(null=True, blank=True, max_length=20000) @@ -166,7 +170,7 @@ def __str__(self): return "Project %s - Environment %s" % (self.project.name, self.name) def natural_key(self): - return (self.api_key,) + return (self.uuid,) def clone( self, name: str, api_key: str = None, clone_feature_states_async: bool = False @@ -179,6 +183,7 @@ def clone( clone = deepcopy(self) clone.id = None clone.name = name + clone.uuid = uuid.uuid4() clone.api_key = api_key if api_key else create_hash() clone.is_creating = True clone.save() diff --git a/api/environments/serializers.py b/api/environments/serializers.py index 4d2836732859..70ac6f9be8cc 100644 --- a/api/environments/serializers.py +++ b/api/environments/serializers.py @@ -44,6 +44,7 @@ class Meta: model = Environment fields = ( "id", + "uuid", "name", "api_key", "description", diff --git a/api/environments/views.py b/api/environments/views.py index b60b8a1ab929..53d1d7ab0b3f 100644 --- a/api/environments/views.py +++ b/api/environments/views.py @@ -8,6 +8,7 @@ from rest_framework import mixins, status, viewsets from rest_framework.decorators import action from rest_framework.exceptions import ValidationError +from rest_framework.generics import get_object_or_404 from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.response import Response @@ -29,6 +30,7 @@ UserObjectPermissionsSerializer, ) from projects.models import Project +from util.uuid import is_valid_uuid from webhooks.mixins import TriggerSampleWebhookMixin from webhooks.webhooks import WebhookType @@ -69,7 +71,7 @@ ), ) class EnvironmentViewSet(viewsets.ModelViewSet): - lookup_field = "api_key" + lookup_field = "api_key" # This is deprecated please uuid permission_classes = [EnvironmentPermissions] def get_serializer_class(self): @@ -91,6 +93,19 @@ def get_serializer_context(self): context["environment"] = self.get_object() return context + def get_object(self): + queryset = self.filter_queryset(self.get_queryset()) + lookup_value = self.kwargs[self.lookup_field] + if is_valid_uuid(lookup_value): + obj = get_object_or_404(queryset, uuid=lookup_value) + else: + obj = get_object_or_404(queryset, api_key=lookup_value) + + # May raise a permission denied + self.check_object_permissions(self.request, obj) + + return obj + def get_queryset(self): if self.action == "list": project_id = self.request.query_params.get( @@ -113,8 +128,14 @@ def get_queryset(self): # Since we don't have the environment at this stage, we would need to query the database # regardless, so it seems worthwhile to just query the database for the latest versions # and use their existence as a proxy to whether v2 feature versioning is enabled. - latest_versions = EnvironmentFeatureVersion.objects.get_latest_versions_by_environment_api_key( - environment_api_key=self.kwargs["api_key"] + + lookup_field_value = self.kwargs["api_key"] + latest_version_kwargs = {"environment_api_key": lookup_field_value} + if is_valid_uuid(lookup_field_value): + latest_version_kwargs = {"environment_uuid": lookup_field_value} + + latest_versions = EnvironmentFeatureVersion.objects.get_latest_versions( + **latest_version_kwargs ) if latest_versions: # if there are latest versions (and hence v2 feature versioning is enabled), then diff --git a/api/features/versioning/managers.py b/api/features/versioning/managers.py index d78131907460..27c541c8419c 100644 --- a/api/features/versioning/managers.py +++ b/api/features/versioning/managers.py @@ -20,13 +20,15 @@ def get_latest_versions_by_environment_id(self, environment_id: int) -> RawQuery """ return self._get_latest_versions(environment_id=environment_id) - def get_latest_versions_by_environment_api_key( - self, environment_api_key: str + def get_latest_versions( + self, environment_api_key: str = None, environment_uuid: str = None ) -> RawQuerySet: """ Get the latest EnvironmentFeatureVersion objects for a given environment. """ - return self._get_latest_versions(environment_api_key=environment_api_key) + return self._get_latest_versions( + environment_api_key=environment_api_key, environment_uuid=environment_uuid + ) def get_latest_versions_as_queryset( self, environment_id: int @@ -46,17 +48,24 @@ def get_latest_versions_as_queryset( ) def _get_latest_versions( - self, environment_id: int = None, environment_api_key: str = None + self, + environment_id: int = None, + environment_api_key: str = None, + environment_uuid: str = None, ) -> RawQuerySet: - assert (environment_id or environment_api_key) and not ( - environment_id and environment_api_key - ), "Must provide exactly one of environment_id or environment_api_key" + assert ( + sum( + bool(x) for x in [environment_id, environment_api_key, environment_uuid] + ) + == 1 + ), "Must provide exactly one of environment_id, environment_api_key, or environment_uuid" return self.raw( get_latest_versions_sql, params={ "environment_id": environment_id, "api_key": environment_api_key, + "environment_uuid": environment_uuid, # TODO: # It seems as though there is a timezone issue when using postgres's # built in now() function, so we pass in the current time from python. diff --git a/api/features/versioning/sql/get_latest_versions.sql b/api/features/versioning/sql/get_latest_versions.sql index 08ca0e8919da..41be31875f05 100644 --- a/api/features/versioning/sql/get_latest_versions.sql +++ b/api/features/versioning/sql/get_latest_versions.sql @@ -26,4 +26,5 @@ inner join environments_environment e on e.id = efv1.environment_id where (%(environment_id)s is not null and efv1.environment_id = %(environment_id)s) - or (%(api_key)s is not null and e.api_key = %(api_key)s); \ No newline at end of file + or (%(api_key)s is not null and e.api_key = %(api_key)s) + or (%(environment_uuid)s is not null and e.uuid = %(environment_uuid)s) diff --git a/api/tests/unit/environments/test_unit_environments_views.py b/api/tests/unit/environments/test_unit_environments_views.py index bbe4102330f7..18812e6e5ee7 100644 --- a/api/tests/unit/environments/test_unit_environments_views.py +++ b/api/tests/unit/environments/test_unit_environments_views.py @@ -34,11 +34,15 @@ from users.models import FFAdminUser +@pytest.mark.parametrize( + "lookup_field", + [lazy_fixture("environment_uuid"), lazy_fixture("environment_client_api_key")], +) def test_retrieve_environment( - admin_client_new: APIClient, environment: Environment + admin_client_new: APIClient, environment: Environment, lookup_field: str ) -> None: # Given - url = reverse("api-v1:environments:environment-detail", args=[environment.api_key]) + url = reverse("api-v1:environments:environment-detail", args=[lookup_field]) # When response = admin_client_new.get(url) @@ -49,6 +53,7 @@ def test_retrieve_environment( response_json = response.json() assert response_json["id"] == environment.id + assert response.json()["uuid"] == str(environment.uuid) assert response_json["name"] == environment.name assert response_json["project"] == environment.project_id assert response_json["api_key"] == environment.api_key @@ -93,17 +98,22 @@ def test_user_with_view_environment_permission_can_retrieve_environment( assert response.status_code == status.HTTP_200_OK +@pytest.mark.parametrize( + "lookup_field", + [lazy_fixture("environment_uuid"), lazy_fixture("environment_client_api_key")], +) def test_can_clone_environment_with_create_environment_permission( - test_user, - test_user_client, - environment, - user_project_permission, + test_user: FFAdminUser, + test_user_client: APIClient, + environment: Environment, + user_project_permission: UserEnvironmentPermission, + lookup_field: str, ) -> None: # Given env_name = "Cloned env" user_project_permission.permissions.add(CREATE_ENVIRONMENT) - url = reverse("api-v1:environments:environment-clone", args=[environment.api_key]) + url = reverse("api-v1:environments:environment-clone", args=[lookup_field]) # When response = test_user_client.post(url, {"name": env_name}) @@ -193,11 +203,16 @@ def test_audit_log_created_when_feature_state_updated( assert AuditLog.objects.first().master_api_key == master_api_key +@pytest.mark.parametrize( + "lookup_field", + [lazy_fixture("environment_uuid"), lazy_fixture("environment_client_api_key")], +) def test_delete_trait_keys_deletes_trait_for_all_users_in_that_environment( environment: Environment, identity: Identity, admin_client_new: APIClient, project: Project, + lookup_field: str, ) -> None: # Given environment2 = Environment.objects.create( @@ -224,7 +239,7 @@ def test_delete_trait_keys_deletes_trait_for_all_users_in_that_environment( url = reverse( "api-v1:environments:environment-delete-traits", - args=[environment.api_key], + args=[lookup_field], ) # When @@ -237,16 +252,19 @@ def test_delete_trait_keys_deletes_trait_for_all_users_in_that_environment( assert Trait.objects.filter(identity=identity2, trait_key=trait_key).exists() +@pytest.mark.parametrize( + "lookup_field", + [lazy_fixture("environment_uuid"), lazy_fixture("environment_client_api_key")], +) def test_environment_user_can_get_their_permissions( staff_client: APIClient, with_environment_permissions: WithEnvironmentPermissionsCallable, environment: Environment, + lookup_field: str, ) -> None: # Given with_environment_permissions([VIEW_ENVIRONMENT]) - url = reverse( - "api-v1:environments:environment-my-permissions", args=[environment.api_key] - ) + url = reverse("api-v1:environments:environment-my-permissions", args=[lookup_field]) # When response = staff_client.get(url) @@ -798,12 +816,18 @@ def test_update_environment_metadata( assert Metadata.objects.filter(id=environment_metadata_b.id).exists() is False +@pytest.mark.parametrize( + "lookup_field", + [lazy_fixture("environment_uuid"), lazy_fixture("environment_client_api_key")], +) def test_audit_log_entry_created_when_environment_updated( - environment: Environment, project: Project, admin_client_new: APIClient + environment: Environment, + project: Project, + admin_client_new: APIClient, + lookup_field: str, ) -> None: # Given - environment = Environment.objects.create(name="Test environment", project=project) - url = reverse("api-v1:environments:environment-detail", args=[environment.api_key]) + url = reverse("api-v1:environments:environment-detail", args=[lookup_field]) banner_text = "production environment be careful" banner_colour = "#FF0000" hide_disabled_flags = True @@ -849,6 +873,10 @@ def test_audit_log_entry_created_when_environment_updated( ) +@pytest.mark.parametrize( + "lookup_field", + [lazy_fixture("environment_uuid"), lazy_fixture("environment_client_api_key")], +) def test_get_document( environment: Environment, project: Project, @@ -856,6 +884,7 @@ def test_get_document( feature: Feature, segment: Segment, with_environment_permissions: WithEnvironmentPermissionsCallable, + lookup_field: str, ) -> None: # Given with_environment_permissions([VIEW_ENVIRONMENT]) @@ -869,9 +898,7 @@ def test_get_document( ) # and the relevant URL to get an environment document - url = reverse( - "api-v1:environments:environment-get-document", args=[environment.api_key] - ) + url = reverse("api-v1:environments:environment-get-document", args=[lookup_field]) # When response = staff_client.get(url) @@ -896,11 +923,16 @@ def test_cannot_get_environment_document_without_permission( assert response.status_code == status.HTTP_403_FORBIDDEN +@pytest.mark.parametrize( + "lookup_field", + [lazy_fixture("environment_uuid"), lazy_fixture("environment_client_api_key")], +) def test_get_all_trait_keys_for_environment_only_returns_distinct_keys( identity: Identity, admin_client_new: APIClient, trait: Trait, environment: Environment, + lookup_field: str, ) -> None: # Given trait_key_one = trait.trait_key @@ -923,9 +955,7 @@ def test_get_all_trait_keys_for_environment_only_returns_distinct_keys( value_type=STRING, ) - url = reverse( - "api-v1:environments:environment-trait-keys", args=[environment.api_key] - ) + url = reverse("api-v1:environments:environment-trait-keys", args=[lookup_field]) # When res = admin_client_new.get(url) @@ -1046,15 +1076,20 @@ def test_partial_environment_update( assert response.status_code == status.HTTP_200_OK +@pytest.mark.parametrize( + "lookup_field", + [lazy_fixture("environment_uuid"), lazy_fixture("environment_client_api_key")], +) def test_cannot_enable_v2_versioning_for_environment_already_enabled( environment_v2_versioning: Environment, admin_client_new: APIClient, mocker: MockerFixture, + lookup_field: str, ) -> None: # Given url = reverse( "api-v1:environments:environment-enable-v2-versioning", - args=[environment_v2_versioning.api_key], + args=[lookup_field], ) mock_enable_v2_versioning = mocker.patch("environments.views.enable_v2_versioning") diff --git a/api/util/uuid.py b/api/util/uuid.py new file mode 100644 index 000000000000..42ca99a1aa99 --- /dev/null +++ b/api/util/uuid.py @@ -0,0 +1,11 @@ +import uuid + + +def is_valid_uuid(value: str) -> bool: + try: + # Attempt to create a UUID object + uuid_obj = uuid.UUID(value) + # Ensure the string matches the canonical UUID format + return str(uuid_obj) == value + except (ValueError, AttributeError, TypeError): + return False