From 65be52b72835ba36075b499fdac59a1ace61b6ec Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 19 Dec 2023 10:23:53 +0000 Subject: [PATCH] feat: Count v2 identity overrides for feature state list view (#3164) --- .../identities/edge_identity_service.py | 9 +- api/environments/dynamodb/dynamodb_wrapper.py | 52 ++-- api/environments/dynamodb/utils.py | 32 +-- api/features/features_service.py | 82 +++++- api/features/views.py | 13 +- api/poetry.lock | 35 ++- api/pyproject.toml | 1 - .../test_edge_api_identities_views.py | 4 +- ...st_unit_dynamodb_environment_v2_wrapper.py | 4 +- .../test_unit_features_features_service.py | 264 ++++++++++++++++-- 10 files changed, 394 insertions(+), 102 deletions(-) diff --git a/api/edge_api/identities/edge_identity_service.py b/api/edge_api/identities/edge_identity_service.py index 0c8d2633e4c0..010ffd204cdc 100644 --- a/api/edge_api/identities/edge_identity_service.py +++ b/api/edge_api/identities/edge_identity_service.py @@ -7,9 +7,12 @@ def get_edge_identity_overrides( - environment_id: int, feature_id: int + environment_id: int, + feature_id: int | None = None, ) -> typing.List[IdentityOverrideV2]: - override_items = ddb_environment_v2_wrapper.get_identity_overrides_by_feature_id( - environment_id=environment_id, feature_id=feature_id + override_items = ( + ddb_environment_v2_wrapper.get_identity_overrides_by_environment_id( + environment_id=environment_id, feature_id=feature_id + ) ) return [IdentityOverrideV2.parse_obj(item) for item in override_items] diff --git a/api/environments/dynamodb/dynamodb_wrapper.py b/api/environments/dynamodb/dynamodb_wrapper.py index 5921e6c96699..208c2bed7728 100644 --- a/api/environments/dynamodb/dynamodb_wrapper.py +++ b/api/environments/dynamodb/dynamodb_wrapper.py @@ -51,33 +51,41 @@ class BaseDynamoWrapper: table_name: str = None - def __init__(self): - self._table: "Table" | None = None - if table_name := self.get_table_name(): - self._table = boto3.resource( - "dynamodb", config=Config(tcp_keepalive=True) - ).Table(table_name) + def __init__(self) -> None: + self._table: typing.Optional["Table"] = None @property - def is_enabled(self) -> bool: - return self._table is not None + def table(self) -> typing.Optional["Table"]: + if not self._table: + self._table = self.get_table() + return self._table - def get_table_name(self): + def get_table_name(self) -> str: return self.table_name + def get_table(self) -> typing.Optional["Table"]: + if table_name := self.get_table_name(): + return boto3.resource("dynamodb", config=Config(tcp_keepalive=True)).Table( + table_name + ) + + @property + def is_enabled(self) -> bool: + return self.table is not None + class DynamoIdentityWrapper(BaseDynamoWrapper): def get_table_name(self) -> str | None: return settings.IDENTITIES_TABLE_NAME_DYNAMO def query_items(self, *args, **kwargs) -> "QueryOutputTableTypeDef": - return self._table.query(*args, **kwargs) + return self.table.query(*args, **kwargs) def put_item(self, identity_dict: dict): - self._table.put_item(Item=identity_dict) + self.table.put_item(Item=identity_dict) def write_identities(self, identities: Iterable["Identity"]): - with self._table.batch_writer() as batch: + with self.table.batch_writer() as batch: for identity in identities: identity_document = map_identity_to_identity_document(identity) # Since sort keys can not be greater than 1024 @@ -90,10 +98,10 @@ def write_identities(self, identities: Iterable["Identity"]): batch.put_item(Item=identity_document) def get_item(self, composite_key: str) -> typing.Optional[dict]: - return self._table.get_item(Key={"composite_key": composite_key}).get("Item") + return self.table.get_item(Key={"composite_key": composite_key}).get("Item") def delete_item(self, composite_key: str): - self._table.delete_item(Key={"composite_key": composite_key}) + self.table.delete_item(Key={"composite_key": composite_key}) def get_item_from_uuid(self, uuid: str) -> dict: filter_expression = Key("identity_uuid").eq(uuid) @@ -200,7 +208,7 @@ class DynamoEnvironmentWrapper(BaseDynamoEnvironmentWrapper): table_name = settings.ENVIRONMENTS_TABLE_NAME_DYNAMO def write_environments(self, environments: Iterable["Environment"]): - with self._table.batch_writer() as writer: + with self.table.batch_writer() as writer: for environment in environments: writer.put_item( Item=map_environment_to_environment_document(environment), @@ -208,7 +216,7 @@ def write_environments(self, environments: Iterable["Environment"]): def get_item(self, api_key: str) -> dict: try: - return self._table.get_item(Key={"api_key": api_key})["Item"] + return self.table.get_item(Key={"api_key": api_key})["Item"] except KeyError as e: raise ObjectDoesNotExist() from e @@ -217,13 +225,13 @@ class DynamoEnvironmentV2Wrapper(BaseDynamoEnvironmentWrapper): def get_table_name(self) -> str | None: return settings.ENVIRONMENTS_V2_TABLE_NAME_DYNAMO - def get_identity_overrides_by_feature_id( + def get_identity_overrides_by_environment_id( self, environment_id: int, - feature_id: int, + feature_id: int | None = None, ) -> typing.List[dict[str, Any]]: try: - response = self._table.query( + response = self.table.query( KeyConditionExpression=Key(ENVIRONMENTS_V2_PARTITION_KEY).eq( str(environment_id), ) @@ -246,7 +254,7 @@ def update_identity_overrides( changeset.to_delete, chunk_size=DYNAMODB_MAX_BATCH_WRITE_ITEM_COUNT, ): - with self._table.batch_writer() as writer: + with self.table.batch_writer() as writer: for identity_override_to_delete in to_delete: writer.delete_item( Key={ @@ -262,7 +270,7 @@ def update_identity_overrides( ) def write_environments(self, environments: Iterable["Environment"]) -> None: - with self._table.batch_writer() as writer: + with self.table.batch_writer() as writer: for environment in environments: writer.put_item( Item=map_environment_to_environment_v2_document(environment), @@ -276,7 +284,7 @@ def write_api_key(self, api_key: "EnvironmentAPIKey"): self.write_api_keys([api_key]) def write_api_keys(self, api_keys: Iterable["EnvironmentAPIKey"]): - with self._table.batch_writer() as writer: + with self.table.batch_writer() as writer: for api_key in api_keys: writer.put_item( Item=map_environment_api_key_to_environment_api_key_document( diff --git a/api/environments/dynamodb/utils.py b/api/environments/dynamodb/utils.py index 40c1f14cbe4f..9f8540eee9b5 100644 --- a/api/environments/dynamodb/utils.py +++ b/api/environments/dynamodb/utils.py @@ -1,23 +1,13 @@ -from multimethod import overload - -# TODO This might require type: ignores in the future, but it's just so nice! - - -@overload -def get_environments_v2_identity_override_document_key() -> str: - return "identity_override:" - - -@overload -def get_environments_v2_identity_override_document_key( # noqa: F811 - feature_id: int, -) -> str: - return f"identity_override:{feature_id}:" - - -@overload -def get_environments_v2_identity_override_document_key( # noqa: F811 - feature_id: int, - identity_uuid: str, +def get_environments_v2_identity_override_document_key( + feature_id: int | None = None, + identity_uuid: str | None = None, ) -> str: + if feature_id is None: + if identity_uuid: + raise ValueError( + "Cannot generate identity override document key without feature_id" + ) + return "identity_override:" + if identity_uuid is None: + return f"identity_override:{feature_id}:" return f"identity_override:{feature_id}:{identity_uuid}" diff --git a/api/features/features_service.py b/api/features/features_service.py index 68ee48e3f58e..59fb2abde08e 100644 --- a/api/features/features_service.py +++ b/api/features/features_service.py @@ -1,24 +1,61 @@ import typing +from concurrent.futures import ThreadPoolExecutor +from edge_api.identities.edge_identity_service import ( + get_edge_identity_overrides, +) from features.dataclasses import EnvironmentFeatureOverridesData from features.versioning.versioning_service import get_environment_flags_list +from projects.models import IdentityOverridesV2MigrationStatus if typing.TYPE_CHECKING: from environments.models import Environment +OverridesData = dict[int, EnvironmentFeatureOverridesData] + + def get_overrides_data( environment: "Environment", -) -> typing.Dict[int, EnvironmentFeatureOverridesData]: +) -> OverridesData: + """ + Get correct overrides counts for a given environment. + + :param project: project to get overrides data for + :return: overrides data getter + """ + project = environment.project + match project.enable_dynamo_db, project.identity_overrides_v2_migration_status: + case True, IdentityOverridesV2MigrationStatus.COMPLETE: + # If v2 migration is complete, count segment overrides from Core + # and identity overrides from DynamoDB. + return get_edge_overrides_data(environment) + case True, _: + # If v2 migration is in progress or not started, we want to count Core overrides, + # but only the segment ones, as the identity ones in DynamoDB are uncountable for v1. + return get_core_overrides_data( + environment, + skip_identity_overrides=True, + ) + case _, _: + # For projects still fully on Core, count all overrides from Core. + return get_core_overrides_data(environment) + + +def get_core_overrides_data( + environment: "Environment", + *, + skip_identity_overrides: bool = False, +) -> OverridesData: """ Get the number of identity / segment overrides in a given environment for each feature in the project. :param environment: the environment to get the overrides data for - :return: dictionary of {feature_id: EnvironmentFeatureOverridesData} + :return OverridesData: dictionary of {feature_id: EnvironmentFeatureOverridesData} """ environment_feature_states_list = get_environment_flags_list(environment) - all_overrides_data = {} + all_overrides_data: OverridesData = {} for feature_state in environment_feature_states_list: env_feature_overrides_data = all_overrides_data.setdefault( @@ -26,8 +63,45 @@ def get_overrides_data( ) if feature_state.feature_segment_id: env_feature_overrides_data.num_segment_overrides += 1 + elif skip_identity_overrides: + continue elif feature_state.identity_id: env_feature_overrides_data.add_identity_override() - all_overrides_data[feature_state.feature_id] = env_feature_overrides_data + + return all_overrides_data + + +def get_edge_overrides_data( + environment: "Environment", +) -> OverridesData: + """ + Get the number of identity / segment overrides in a given environment for each feature in the + project. + Retrieve identity override data from DynamoDB. + + :param environment: the environment to get the overrides data for + :return OverridesData: dictionary of {feature_id: EnvironmentFeatureOverridesData} + """ + with ThreadPoolExecutor() as executor: + get_environment_flags_list_future = executor.submit( + get_environment_flags_list, + environment, + ) + get_overrides_data_future = executor.submit( + get_edge_identity_overrides, + environment_id=environment.id, + ) + all_overrides_data: OverridesData = {} + + for feature_state in get_environment_flags_list_future.result(): + env_feature_overrides_data = all_overrides_data.setdefault( + feature_state.feature_id, EnvironmentFeatureOverridesData() + ) + if feature_state.feature_segment_id: + env_feature_overrides_data.num_segment_overrides += 1 + for identity_override in get_overrides_data_future.result(): + all_overrides_data[ + identity_override.feature_state.feature.id + ].add_identity_override() return all_overrides_data diff --git a/api/features/views.py b/api/features/views.py index 0a651414bfe1..75ca0b18df3f 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -152,13 +152,12 @@ def perform_destroy(self, instance): def get_serializer_context(self): context = super().get_serializer_context() - if self.kwargs.get("project_pk"): - context.update( - project=get_object_or_404( - Project.objects.all(), pk=self.kwargs["project_pk"] - ), - user=self.request.user, - ) + context.update( + project=get_object_or_404( + Project.objects.all(), pk=self.kwargs["project_pk"] + ), + user=self.request.user, + ) if self.action == "list" and "environment" in self.request.query_params: environment = get_object_or_404( Environment, id=self.request.query_params["environment"] diff --git a/api/poetry.lock b/api/poetry.lock index fb3055b781cd..4e22729961a2 100644 --- a/api/poetry.lock +++ b/api/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.7.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.5.1 and should not be changed by hand. [[package]] name = "aiohttp" @@ -2094,6 +2094,16 @@ files = [ {file = "MarkupSafe-2.1.3-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:5bbe06f8eeafd38e5d0a4894ffec89378b6c6a625ff57e3028921f8ff59318ac"}, {file = "MarkupSafe-2.1.3-cp311-cp311-win32.whl", hash = "sha256:dd15ff04ffd7e05ffcb7fe79f1b98041b8ea30ae9234aed2a9168b5797c3effb"}, {file = "MarkupSafe-2.1.3-cp311-cp311-win_amd64.whl", hash = "sha256:134da1eca9ec0ae528110ccc9e48041e0828d79f24121a1a146161103c76e686"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-macosx_10_9_universal2.whl", hash = "sha256:f698de3fd0c4e6972b92290a45bd9b1536bffe8c6759c62471efaa8acb4c37bc"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:aa57bd9cf8ae831a362185ee444e15a93ecb2e344c8e52e4d721ea3ab6ef1823"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:ffcc3f7c66b5f5b7931a5aa68fc9cecc51e685ef90282f4a82f0f5e9b704ad11"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:47d4f1c5f80fc62fdd7777d0d40a2e9dda0a05883ab11374334f6c4de38adffd"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:1f67c7038d560d92149c060157d623c542173016c4babc0c1913cca0564b9939"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_aarch64.whl", hash = "sha256:9aad3c1755095ce347e26488214ef77e0485a3c34a50c5a5e2471dff60b9dd9c"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_i686.whl", hash = "sha256:14ff806850827afd6b07a5f32bd917fb7f45b046ba40c57abdb636674a8b559c"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8f9293864fe09b8149f0cc42ce56e3f0e54de883a9de90cd427f191c346eb2e1"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-win32.whl", hash = "sha256:715d3562f79d540f251b99ebd6d8baa547118974341db04f5ad06d5ea3eb8007"}, + {file = "MarkupSafe-2.1.3-cp312-cp312-win_amd64.whl", hash = "sha256:1b8dd8c3fd14349433c79fa8abeb573a55fc0fdd769133baac1f5e07abf54aeb"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:8e254ae696c88d98da6555f5ace2279cf7cd5b3f52be2b5cf97feafe883b58d2"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:cb0932dc158471523c9637e807d9bfb93e06a95cbf010f1a38b98623b929ef2b"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:9402b03f1a1b4dc4c19845e5c749e3ab82d5078d16a2a4c2cd2df62d57bb0707"}, @@ -2335,17 +2345,6 @@ files = [ {file = "multidict-6.0.4.tar.gz", hash = "sha256:3666906492efb76453c0e7b97f2cf459b0682e7402c0489a95484965dbc1da49"}, ] -[[package]] -name = "multimethod" -version = "1.10" -description = "Multiple argument dispatching." -optional = false -python-versions = ">=3.8" -files = [ - {file = "multimethod-1.10-py3-none-any.whl", hash = "sha256:afd84da9c3d0445c84f827e4d63ad42d17c6d29b122427c6dee9032ac2d2a0d4"}, - {file = "multimethod-1.10.tar.gz", hash = "sha256:daa45af3fe257f73abb69673fd54ddeaf31df0eb7363ad6e1251b7c9b192d8c5"}, -] - [[package]] name = "mypy-boto3-dynamodb" version = "1.33.0" @@ -3427,6 +3426,7 @@ files = [ {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:69b023b2b4daa7548bcfbd4aa3da05b3a74b772db9e23b982788168117739938"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:81e0b275a9ecc9c0c0c07b4b90ba548307583c125f54d5b6946cfee6360c733d"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:ba336e390cd8e4d1739f42dfe9bb83a3cc2e80f567d8805e11b46f4a943f5515"}, + {file = "PyYAML-6.0.1-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:326c013efe8048858a6d312ddd31d56e468118ad4cdeda36c719bf5bb6192290"}, {file = "PyYAML-6.0.1-cp310-cp310-win32.whl", hash = "sha256:bd4af7373a854424dabd882decdc5579653d7868b8fb26dc7d0e99f823aa5924"}, {file = "PyYAML-6.0.1-cp310-cp310-win_amd64.whl", hash = "sha256:fd1592b3fdf65fff2ad0004b5e363300ef59ced41c2e6b3a99d4089fa8c5435d"}, {file = "PyYAML-6.0.1-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:6965a7bc3cf88e5a1c3bd2e0b5c22f8d677dc88a455344035f03399034eb3007"}, @@ -3434,8 +3434,15 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:42f8152b8dbc4fe7d96729ec2b99c7097d656dc1213a3229ca5383f973a5ed6d"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:062582fca9fabdd2c8b54a3ef1c978d786e0f6b3a1510e0ac93ef59e0ddae2bc"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:d2b04aac4d386b172d5b9692e2d2da8de7bfb6c387fa4f801fbf6fb2e6ba4673"}, + {file = "PyYAML-6.0.1-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:e7d73685e87afe9f3b36c799222440d6cf362062f78be1013661b00c5c6f678b"}, {file = "PyYAML-6.0.1-cp311-cp311-win32.whl", hash = "sha256:1635fd110e8d85d55237ab316b5b011de701ea0f29d07611174a1b42f1444741"}, {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, + {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, + {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, + {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, + {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, + {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, + {file = "PyYAML-6.0.1-cp312-cp312-win_amd64.whl", hash = "sha256:0d3304d8c0adc42be59c5f8a4d9e3d7379e6955ad754aa9d6ab7a398b59dd1df"}, {file = "PyYAML-6.0.1-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:50550eb667afee136e9a77d6dc71ae76a44df8b3e51e41b77f6de2932bfe0f47"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:1fe35611261b29bd1de0070f0b2f47cb6ff71fa6595c077e42bd0c419fa27b98"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:704219a11b772aea0d8ecd7058d0082713c3562b4e271b849ad7dc4a5c90c13c"}, @@ -3452,6 +3459,7 @@ files = [ {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a0cd17c15d3bb3fa06978b4e8958dcdc6e0174ccea823003a106c7d4d7899ac5"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:28c119d996beec18c05208a8bd78cbe4007878c6dd15091efb73a30e90539696"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:7e07cbde391ba96ab58e532ff4803f79c4129397514e1413a7dc761ccd755735"}, + {file = "PyYAML-6.0.1-cp38-cp38-musllinux_1_1_x86_64.whl", hash = "sha256:49a183be227561de579b4a36efbb21b3eab9651dd81b1858589f796549873dd6"}, {file = "PyYAML-6.0.1-cp38-cp38-win32.whl", hash = "sha256:184c5108a2aca3c5b3d3bf9395d50893a7ab82a38004c8f61c258d4428e80206"}, {file = "PyYAML-6.0.1-cp38-cp38-win_amd64.whl", hash = "sha256:1e2722cc9fbb45d9b87631ac70924c11d3a401b2d7f410cc0e3bbf249f2dca62"}, {file = "PyYAML-6.0.1-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:9eb6caa9a297fc2c2fb8862bc5370d0303ddba53ba97e71f08023b6cd73d16a8"}, @@ -3459,6 +3467,7 @@ files = [ {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:5773183b6446b2c99bb77e77595dd486303b4faab2b086e7b17bc6bef28865f6"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:b786eecbdf8499b9ca1d697215862083bd6d2a99965554781d0d8d1ad31e13a0"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:bc1bf2925a1ecd43da378f4db9e4f799775d6367bdb94671027b73b393a7c42c"}, + {file = "PyYAML-6.0.1-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:04ac92ad1925b2cff1db0cfebffb6ffc43457495c9b3c39d3fcae417d7125dc5"}, {file = "PyYAML-6.0.1-cp39-cp39-win32.whl", hash = "sha256:faca3bdcf85b2fc05d06ff3fbc1f83e1391b3e724afa3feba7d13eeab355484c"}, {file = "PyYAML-6.0.1-cp39-cp39-win_amd64.whl", hash = "sha256:510c9deebc5c0225e8c96813043e62b680ba2f9c50a08d3724c7f28a747d1486"}, {file = "PyYAML-6.0.1.tar.gz", hash = "sha256:bfdf460b1736c775f2ba9f6a92bca30bc2095067b8a9d77876d1fad6cc3b4a43"}, @@ -4408,4 +4417,4 @@ requests = ">=2.7,<3.0" [metadata] lock-version = "2.0" python-versions = ">=3.10,<3.12" -content-hash = "c4f455c206a45222736db051afd1ab7f00a37748ac7234253d4225b8b60764f7" +content-hash = "aec2b48884672a3fe24e327cd716a142735998132ff75fd80ea8286a736f9fd4" diff --git a/api/pyproject.toml b/api/pyproject.toml index d08402f83319..1100faaca11f 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -103,7 +103,6 @@ pydantic = "~1.10.9" pyngo = "~1.6.0" flagsmith = "^3.4.0" python-gnupg = "^0.5.1" -multimethod = "^1.10" [tool.poetry.group.auth-controller] optional = true diff --git a/api/tests/unit/edge_api/identities/test_edge_api_identities_views.py b/api/tests/unit/edge_api/identities/test_edge_api_identities_views.py index 900d3173b302..3ecdac6cab20 100644 --- a/api/tests/unit/edge_api/identities/test_edge_api_identities_views.py +++ b/api/tests/unit/edge_api/identities/test_edge_api_identities_views.py @@ -119,7 +119,7 @@ def test_get_edge_identity_overrides_for_a_feature( mock_dynamodb_wrapper, ) - mock_dynamodb_wrapper.get_identity_overrides_by_feature_id.return_value = [ + mock_dynamodb_wrapper.get_identity_overrides_by_environment_id.return_value = [ edge_identity_override_document, edge_identity_override_document_2, ] @@ -159,7 +159,7 @@ def test_get_edge_identity_overrides_for_a_feature( }, } - mock_dynamodb_wrapper.get_identity_overrides_by_feature_id.assert_called_once_with( + mock_dynamodb_wrapper.get_identity_overrides_by_environment_id.assert_called_once_with( environment_id=environment.id, feature_id=feature.id ) diff --git a/api/tests/unit/environments/dynamodb/test_unit_dynamodb_environment_v2_wrapper.py b/api/tests/unit/environments/dynamodb/test_unit_dynamodb_environment_v2_wrapper.py index 52086b3a259b..805990e9a800 100644 --- a/api/tests/unit/environments/dynamodb/test_unit_dynamodb_environment_v2_wrapper.py +++ b/api/tests/unit/environments/dynamodb/test_unit_dynamodb_environment_v2_wrapper.py @@ -17,7 +17,7 @@ ) -def test_environment_v2_wrapper__get_identity_overrides__return_expected( +def test_environment_v2_wrapper__get_identity_overrides_by_environment_id__return_expected( settings: SettingsWrapper, environment: Environment, flagsmith_environments_v2_table: Table, @@ -43,7 +43,7 @@ def test_environment_v2_wrapper__get_identity_overrides__return_expected( flagsmith_environments_v2_table.put_item(Item=environment_document) # When - results = wrapper.get_identity_overrides_by_feature_id( + results = wrapper.get_identity_overrides_by_environment_id( environment_id=environment.id, feature_id=feature.id, ) diff --git a/api/tests/unit/features/test_unit_features_features_service.py b/api/tests/unit/features/test_unit_features_features_service.py index a0b79aeff5ea..a02732840e5f 100644 --- a/api/tests/unit/features/test_unit_features_features_service.py +++ b/api/tests/unit/features/test_unit_features_features_service.py @@ -1,35 +1,182 @@ +from typing import TYPE_CHECKING +from unittest.mock import ANY + +import pytest + +from edge_api.identities.models import EdgeIdentity from environments.identities.models import Identity -from features.features_service import get_overrides_data +from features.features_service import ( + get_core_overrides_data, + get_edge_overrides_data, + get_overrides_data, +) from features.models import Feature, FeatureSegment, FeatureState +from projects.models import IdentityOverridesV2MigrationStatus +from util.mappers.engine import ( + map_feature_state_to_engine, + map_identity_to_engine, +) +if TYPE_CHECKING: + from pytest_mock import MockerFixture -def test_feature_get_overrides_data( - feature, - environment, - identity, - segment, - feature_segment, - identity_featurestate, - segment_featurestate, -): - # Given - # we create some other features with overrides to ensure we're only getting data - # for each individual feature - feature_2 = Feature.objects.create(project=feature.project, name="feature_2") - FeatureState.objects.create( - feature=feature_2, environment=environment, identity=identity + from environments.dynamodb.dynamodb_wrapper import ( + DynamoEnvironmentV2Wrapper, + DynamoIdentityWrapper, ) + from environments.models import Environment + from segments.models import Segment + - feature_3 = Feature.objects.create(project=feature.project, name="feature_3") - feature_segment_for_feature_3 = FeatureSegment.objects.create( - feature=feature_3, segment=segment, environment=environment +@pytest.fixture +def distinct_segment_featurestate( + environment: "Environment", + segment: "Segment", +) -> FeatureState: + feature = Feature.objects.create( + project=environment.project, name="distinct_feature_1" ) - FeatureState.objects.create( - feature=feature_3, + feature_segment = FeatureSegment.objects.create( + feature=feature, segment=segment, environment=environment + ) + return FeatureState.objects.create( + feature=feature, environment=environment, - feature_segment=feature_segment_for_feature_3, + feature_segment=feature_segment, + ) + + +@pytest.fixture +def distinct_identity_featurestate( + environment: "Environment", + identity: Identity, +) -> FeatureState: + feature = Feature.objects.create( + project=environment.project, name="distinct_feature_2" + ) + return FeatureState.objects.create( + feature=feature, environment=environment, identity=identity + ) + + +@pytest.mark.parametrize( + "enable_dynamo_db, identity_overrides_v2_migration_status, expected_overrides_getter_name, expected_kwargs", + [ + ( + True, + IdentityOverridesV2MigrationStatus.NOT_STARTED, + "get_core_overrides_data", + {"skip_identity_overrides": True}, + ), + ( + True, + IdentityOverridesV2MigrationStatus.IN_PROGRESS, + "get_core_overrides_data", + {"skip_identity_overrides": True}, + ), + ( + True, + IdentityOverridesV2MigrationStatus.COMPLETE, + "get_edge_overrides_data", + {}, + ), + ( + False, + ANY, + "get_core_overrides_data", + {}, + ), + ], +) +def test_feature_get_overrides_data__call_expected( + mocker: "MockerFixture", + environment: "Environment", + enable_dynamo_db: bool, + identity_overrides_v2_migration_status: str, + expected_overrides_getter_name: str, + expected_kwargs: dict[str, bool], +) -> None: + # Given + mocked_override_getters = { + "get_core_overrides_data": mocker.patch( + "features.features_service.get_core_overrides_data", + autospec=True, + ), + "get_edge_overrides_data": mocker.patch( + "features.features_service.get_edge_overrides_data", + autospec=True, + ), + } + environment.project.enable_dynamo_db = enable_dynamo_db + environment.project.identity_overrides_v2_migration_status = ( + identity_overrides_v2_migration_status + ) + + # When + get_overrides_data(environment) + + # Then + mocked_override_getters.pop(expected_overrides_getter_name).assert_called_once_with( + environment, + **expected_kwargs, + ) + [remaining_override_getter_mock] = mocked_override_getters.values() + remaining_override_getter_mock.assert_not_called() + + +@pytest.mark.parametrize( + "identity_overrides_v2_migration_status", + [ + IdentityOverridesV2MigrationStatus.NOT_STARTED, + IdentityOverridesV2MigrationStatus.IN_PROGRESS, + ], +) +def test_feature_get_overrides_data__edge_project_not_migrated_to_v2__return_expected( + environment: "Environment", + distinct_identity_featurestate: FeatureState, + distinct_segment_featurestate: FeatureState, + identity_overrides_v2_migration_status: str, +) -> None: + # Given + environment.project.enable_dynamo_db = True + environment.project.identity_overrides_v2_migration_status = ( + identity_overrides_v2_migration_status ) + # When + overrides_data = get_overrides_data(environment) + + # Then + assert ( + overrides_data[distinct_segment_featurestate.feature_id].num_segment_overrides + == 1 + ) + assert ( + overrides_data[distinct_segment_featurestate.feature_id].num_identity_overrides + is None + ) + assert ( + overrides_data[distinct_identity_featurestate.feature_id].num_segment_overrides + == 0 + ) + assert ( + overrides_data[distinct_identity_featurestate.feature_id].num_identity_overrides + is None + ) + + +def test_feature_get_core_overrides_data( + feature: Feature, + environment: "Environment", + identity: Identity, + segment: "Segment", + feature_segment: "FeatureSegment", + identity_featurestate: FeatureState, + segment_featurestate: FeatureState, + distinct_segment_featurestate: FeatureState, + distinct_identity_featurestate: FeatureState, +) -> None: + # Given # and an override for another identity that has been deleted another_identity = Identity.objects.create( identifier="another-identity", environment=environment @@ -40,14 +187,77 @@ def test_feature_get_overrides_data( fs_to_delete.delete() # When - overrides_data = get_overrides_data(environment) + overrides_data = get_core_overrides_data(environment) # Then assert overrides_data[feature.id].num_identity_overrides == 1 assert overrides_data[feature.id].num_segment_overrides == 1 - assert overrides_data[feature_2.id].num_identity_overrides == 1 - assert overrides_data[feature_2.id].num_segment_overrides == 0 + assert ( + overrides_data[distinct_identity_featurestate.feature.id].num_identity_overrides + == 1 + ) + assert ( + overrides_data[distinct_identity_featurestate.feature.id].num_segment_overrides + == 0 + ) + + assert ( + overrides_data[distinct_segment_featurestate.feature.id].num_identity_overrides + is None + ) + assert ( + overrides_data[distinct_segment_featurestate.feature.id].num_segment_overrides + == 1 + ) - assert overrides_data[feature_3.id].num_identity_overrides is None - assert overrides_data[feature_3.id].num_segment_overrides == 1 + +@pytest.mark.django_db(transaction=True) +def test_feature_get_edge_overrides_data( + feature: Feature, + environment: "Environment", + identity: Identity, + segment: "Segment", + feature_segment: "FeatureSegment", + identity_featurestate: FeatureState, + segment_featurestate: FeatureState, + distinct_segment_featurestate: FeatureState, + distinct_identity_featurestate: FeatureState, + dynamodb_identity_wrapper: "DynamoIdentityWrapper", + dynamodb_wrapper_v2: "DynamoEnvironmentV2Wrapper", +) -> None: + # Given + # replicate identity to Edge + edge_identity = EdgeIdentity(map_identity_to_engine(identity, with_overrides=False)) + edge_identity.add_feature_override( + map_feature_state_to_engine(identity_featurestate), + ) + edge_identity.add_feature_override( + map_feature_state_to_engine(distinct_identity_featurestate), + ) + edge_identity.save() + + # When + overrides_data = get_edge_overrides_data(environment) + + # Then + assert overrides_data[feature.id].num_identity_overrides == 1 + assert overrides_data[feature.id].num_segment_overrides == 1 + + assert ( + overrides_data[distinct_identity_featurestate.feature.id].num_identity_overrides + == 1 + ) + assert ( + overrides_data[distinct_identity_featurestate.feature.id].num_segment_overrides + == 0 + ) + + assert ( + overrides_data[distinct_segment_featurestate.feature.id].num_identity_overrides + is None + ) + assert ( + overrides_data[distinct_segment_featurestate.feature.id].num_segment_overrides + == 1 + )