diff --git a/api/app_analytics/mappers.py b/api/app_analytics/mappers.py index 8beb284e62ad..2a6bcd3fc11c 100644 --- a/api/app_analytics/mappers.py +++ b/api/app_analytics/mappers.py @@ -24,7 +24,7 @@ TrackFeatureEvaluationsByEnvironmentData, TrackFeatureEvaluationsByEnvironmentKwargs, ) -from integrations.flagsmith.client import get_client +from integrations.flagsmith.client import get_openfeature_client def map_user_agent_to_sdk_user_agent(value: str) -> str | None: @@ -168,10 +168,9 @@ def map_input_labels_to_labels(input_labels: InputLabels) -> Labels: def map_request_to_labels(request: HttpRequest) -> Labels: - if not ( - get_client("local", local_eval=True) - .get_environment_flags() - .is_feature_enabled("sdk_metrics_labels") + if not get_openfeature_client().get_boolean_value( + "sdk_metrics_labels", + default_value=False, ): return {} input_labels: InputLabels = _RequestHeaderLabelsModel.model_validate( diff --git a/api/conftest.py b/api/conftest.py index 09b61c539016..5dcaa80efcf4 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -5,6 +5,7 @@ from unittest.mock import MagicMock import boto3 +import openfeature.api as openfeature_api import pytest from common.environments.permissions import ( MANAGE_IDENTITIES, @@ -19,10 +20,9 @@ from django.db.backends.base.creation import TEST_DATABASE_PREFIX from django.test.utils import setup_databases from flag_engine.segments.constants import EQUAL -from flagsmith import Flagsmith -from flagsmith.models import Flags from moto import mock_dynamodb # type: ignore[import-untyped] from mypy_boto3_dynamodb.service_resource import DynamoDBServiceResource, Table +from openfeature.provider.in_memory_provider import InMemoryFlag, InMemoryProvider from pyfakefs.fake_filesystem import FakeFilesystem from pytest import FixtureRequest from pytest_django.fixtures import SettingsWrapper @@ -50,6 +50,7 @@ from features.value_types import STRING from features.versioning.tasks import enable_v2_versioning from features.workflows.core.models import ChangeRequest +from integrations.flagsmith.client import DEFAULT_OPENFEATURE_DOMAIN from integrations.github.models import GithubConfiguration, GitHubRepository from metadata.models import ( Metadata, @@ -1278,31 +1279,30 @@ def set_github_webhook_secret() -> None: @pytest.fixture() def enable_features( mocker: MockerFixture, -) -> EnableFeaturesFixture: +) -> typing.Generator[EnableFeaturesFixture, None, None]: """ This fixture returns a callable that allows us to enable any Flagsmith feature flag(s) in tests. - Relevant issue for improving this: https://github.com/Flagsmith/flagsmith-python-client/issues/135 + Uses OpenFeature's InMemoryProvider to set up enabled flags, then patches the + module-level client so that all call-sites pick up the test provider. """ def _enable_features(*expected_feature_names: str) -> None: - def _is_feature_enabled(feature_name: str) -> bool: - return feature_name in expected_feature_names - - mock_flags = mocker.MagicMock(spec=Flags) - mock_flags.is_feature_enabled.side_effect = _is_feature_enabled - mock_flagsmith = mocker.MagicMock(spec=Flagsmith) - mock_flagsmith.get_identity_flags.return_value = mock_flags - mock_flagsmith.get_environment_flags.return_value = mock_flags - mock_clients = mocker.MagicMock(spec=dict) - mock_clients.__getitem__.return_value = mock_flagsmith - - mocker.patch( - "integrations.flagsmith.client._flagsmith_clients", - new=mock_clients, + flags = { + name: InMemoryFlag( + variants={"enabled": True}, + default_variant="enabled", + ) + for name in expected_feature_names + } + openfeature_api.set_provider( + InMemoryProvider(flags), + domain=DEFAULT_OPENFEATURE_DOMAIN, ) - return _enable_features + yield _enable_features + + openfeature_api.clear_providers() @pytest.fixture(autouse=True) diff --git a/api/environments/dynamodb/wrappers/environment_wrapper.py b/api/environments/dynamodb/wrappers/environment_wrapper.py index 3ac874df39da..8abe96a89d2b 100644 --- a/api/environments/dynamodb/wrappers/environment_wrapper.py +++ b/api/environments/dynamodb/wrappers/environment_wrapper.py @@ -22,7 +22,7 @@ flagsmith_dynamo_environment_document_compression_ratio, flagsmith_dynamo_environment_document_size_bytes, ) -from integrations.flagsmith.client import get_client +from integrations.flagsmith.client import get_openfeature_client from util.mappers import ( map_environment_to_compressed_environment_document, map_environment_to_compressed_environment_v2_document, @@ -63,7 +63,7 @@ def _map_compressed_environment_document( ) -> "CompressedEnvironmentDocument": ... def _write_environments(self, environments: Iterable["Environment"]) -> None: - flagsmith_client = get_client("local", local_eval=True) + openfeature_client = get_openfeature_client() prefetch_related_objects( environments, "project__organisation", @@ -74,10 +74,11 @@ def _write_environments(self, environments: Iterable["Environment"]) -> None: with self.table.batch_writer() as writer: for environment in environments: organisation = environment.project.organisation - if flagsmith_client.get_identity_flags( - organisation.flagsmith_identifier, - traits=organisation.flagsmith_on_flagsmith_api_traits, - ).is_feature_enabled("compress_dynamo_documents"): + if openfeature_client.get_boolean_value( + "compress_dynamo_documents", + default_value=False, + evaluation_context=organisation.openfeature_evaluation_context, + ): result = self._map_compressed_environment_document(environment) writer.put_item(Item=result.document) diff --git a/api/environments/models.py b/api/environments/models.py index d5b27ea318db..0de6659a272d 100644 --- a/api/environments/models.py +++ b/api/environments/models.py @@ -51,7 +51,7 @@ ) from features.models import Feature, FeatureSegment, FeatureState from features.multivariate.models import MultivariateFeatureStateValue -from integrations.flagsmith.client import get_client +from integrations.flagsmith.client import get_openfeature_client from metadata.models import Metadata from projects.models import Project from segments.models import Segment @@ -207,13 +207,12 @@ def enable_v2_versioning(self) -> None: # we don't want to disable it based on the flag state. return - flagsmith_client = get_client("local", local_eval=True) organisation = self.project.organisation - enable_v2_versioning = flagsmith_client.get_identity_flags( - organisation.flagsmith_identifier, - traits=organisation.flagsmith_on_flagsmith_api_traits, - ).is_feature_enabled("enable_feature_versioning_for_new_environments") - self.use_v2_feature_versioning = enable_v2_versioning + self.use_v2_feature_versioning = get_openfeature_client().get_boolean_value( + "enable_feature_versioning_for_new_environments", + default_value=False, + evaluation_context=organisation.openfeature_evaluation_context, + ) def __str__(self): # type: ignore[no-untyped-def] return "Project %s - Environment %s" % (self.project.name, self.name) diff --git a/api/features/views.py b/api/features/views.py index bbf252daf76f..70f2b47c61aa 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -62,7 +62,7 @@ NestedEnvironmentPermissions, ) from features.value_types import BOOLEAN, INTEGER, STRING -from integrations.flagsmith.client import get_client +from integrations.flagsmith.client import get_openfeature_client from projects.code_references.services import ( annotate_feature_queryset_with_code_references_summary, ) @@ -222,12 +222,11 @@ def get_queryset(self): # type: ignore[no-untyped-def] # TODO: Delete this after https://github.com/flagsmith/flagsmith/issues/6832 is resolved organisation = project.organisation - flagsmith_client = get_client("local", local_eval=True) - flags = flagsmith_client.get_identity_flags( - organisation.flagsmith_identifier, - traits=organisation.flagsmith_on_flagsmith_api_traits, - ) - if flags.is_feature_enabled("code_references_ui_stats"): + if get_openfeature_client().get_boolean_value( + "code_references_ui_stats", + default_value=False, + evaluation_context=organisation.openfeature_evaluation_context, + ): queryset = annotate_feature_queryset_with_code_references_summary(queryset) else: queryset = queryset.annotate( diff --git a/api/integrations/flagsmith/client.py b/api/integrations/flagsmith/client.py index ffb518592dba..b65494100527 100644 --- a/api/integrations/flagsmith/client.py +++ b/api/integrations/flagsmith/client.py @@ -1,46 +1,61 @@ """ -Wrapper module for the flagsmith client to implement singleton behaviour and provide some -additional logic by wrapping the client. +OpenFeature client wrapper for Flagsmith on Flagsmith feature evaluation. Usage: ``` -environment_flags = get_client().get_environment_flags() -identity_flags = get_client().get_identity_flags() +from integrations.flagsmith.client import get_openfeature_client + +client = get_openfeature_client() +enabled = client.get_boolean_value( + "flag_name", default_value=False, evaluation_context=ctx +) ``` """ import typing +import openfeature.api as openfeature_api from django.conf import settings from flagsmith import Flagsmith from flagsmith.offline_handlers import LocalFileHandler +from openfeature.client import OpenFeatureClient +from openfeature.provider import ProviderStatus +from openfeature_flagsmith.provider import FlagsmithProvider from integrations.flagsmith.exceptions import FlagsmithIntegrationError from integrations.flagsmith.flagsmith_service import ENVIRONMENT_JSON_PATH -_flagsmith_clients: dict[str, Flagsmith] = {} +DEFAULT_OPENFEATURE_DOMAIN = "flagsmith-api" -def get_client(name: str = "default", local_eval: bool = False) -> Flagsmith: - global _flagsmith_clients +def get_openfeature_client( + domain: str = DEFAULT_OPENFEATURE_DOMAIN, + **flagsmith_kwargs: typing.Any, +) -> OpenFeatureClient: + openfeature_client = openfeature_api.get_client(domain=domain) + if openfeature_client.get_provider_status() != ProviderStatus.READY: + initialise_provider(domain, **(flagsmith_kwargs or get_provider_kwargs())) + return openfeature_client - try: - _flagsmith_client = _flagsmith_clients[name] - except (KeyError, TypeError): - kwargs = _get_client_kwargs() - kwargs["enable_local_evaluation"] = local_eval - _flagsmith_client = Flagsmith(**kwargs) - _flagsmith_clients[name] = _flagsmith_client - return _flagsmith_client +def initialise_provider( + domain: str = DEFAULT_OPENFEATURE_DOMAIN, + **kwargs: typing.Any, +) -> None: + flagsmith_client = Flagsmith(**kwargs) + provider = FlagsmithProvider(client=flagsmith_client) + openfeature_api.set_provider(provider, domain=domain) -def _get_client_kwargs() -> dict[str, typing.Any]: - _default_kwargs = {"offline_handler": LocalFileHandler(ENVIRONMENT_JSON_PATH)} +def get_provider_kwargs() -> dict[str, typing.Any]: + common_kwargs: dict[str, typing.Any] = { + "offline_handler": LocalFileHandler(ENVIRONMENT_JSON_PATH), + "enable_local_evaluation": True, + } if settings.FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE: - return {"offline_mode": True, **_default_kwargs} + return {"offline_mode": True, **common_kwargs} elif ( settings.FLAGSMITH_ON_FLAGSMITH_SERVER_KEY and settings.FLAGSMITH_ON_FLAGSMITH_SERVER_API_URL @@ -48,7 +63,7 @@ def _get_client_kwargs() -> dict[str, typing.Any]: return { "environment_key": settings.FLAGSMITH_ON_FLAGSMITH_SERVER_KEY, "api_url": settings.FLAGSMITH_ON_FLAGSMITH_SERVER_API_URL, - **_default_kwargs, + **common_kwargs, } raise FlagsmithIntegrationError( diff --git a/api/organisations/models.py b/api/organisations/models.py index 95168db04064..bc24160e7a1c 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -14,6 +14,7 @@ LifecycleModelMixin, hook, ) +from openfeature.evaluation_context import EvaluationContext from simple_history.models import HistoricalRecords # type: ignore[import-untyped] from core.models import SoftDeleteExportableModel @@ -52,7 +53,6 @@ ) from organisations.subscriptions.metadata import BaseSubscriptionMetadata from organisations.subscriptions.xero.metadata import XeroSubscriptionMetadata -from util.engine_models.identities.traits.types import TraitValue from webhooks.models import AbstractBaseExportableWebhookModel environment_cache = caches[settings.ENVIRONMENT_CACHE_NAME] @@ -123,16 +123,15 @@ def has_enterprise_subscription(self) -> bool: return self.is_paid and self.subscription.is_enterprise @property - def flagsmith_identifier(self): # type: ignore[no-untyped-def] - return f"org.{self.id}" - - @property - def flagsmith_on_flagsmith_api_traits(self) -> dict[str, TraitValue]: - return { - "organisation.id": self.id, - "organisation.name": self.name, - "subscription.plan": self.subscription.plan, - } + def openfeature_evaluation_context(self) -> EvaluationContext: + return EvaluationContext( + targeting_key=f"org.{self.id}", + attributes={ + "organisation.id": self.id, + "organisation.name": self.name, + "subscription.plan": self.subscription.plan or "", + }, + ) def over_plan_seats_limit(self, additional_seats: int = 0): # type: ignore[no-untyped-def] if self.has_paid_subscription(): diff --git a/api/organisations/task_helpers.py b/api/organisations/task_helpers.py index fbfc8cb24dd9..ee00b648ec94 100644 --- a/api/organisations/task_helpers.py +++ b/api/organisations/task_helpers.py @@ -10,7 +10,7 @@ from app_analytics.analytics_db_service import get_total_events_count from app_analytics.influxdb_wrapper import get_current_api_usage from core.helpers import get_current_site_url -from integrations.flagsmith.client import get_client +from integrations.flagsmith.client import get_openfeature_client from organisations.models import ( Organisation, OrganisationAPIUsageNotification, @@ -128,13 +128,13 @@ def handle_api_usage_notification_for_organisation(organisation: Organisation) - allowed_api_calls = subscription_cache.allowed_30d_api_calls - flagsmith_client = get_client("local", local_eval=True) - flags = flagsmith_client.get_identity_flags( - organisation.flagsmith_identifier, - traits=organisation.flagsmith_on_flagsmith_api_traits, - ) + openfeature_client = get_openfeature_client() # TODO: Default to get_total_events_count — https://github.com/Flagsmith/flagsmith/issues/6985 - if flags.is_feature_enabled("get_current_api_usage_deprecated"): # pragma: no cover + if openfeature_client.get_boolean_value( + "get_current_api_usage_deprecated", + default_value=False, + evaluation_context=organisation.openfeature_evaluation_context, + ): # pragma: no cover api_usage = get_total_events_count(organisation, period_starts_at) else: api_usage = get_current_api_usage(organisation.id, period_starts_at) diff --git a/api/organisations/tasks.py b/api/organisations/tasks.py index 43c5dca0b278..6a2a0079b017 100644 --- a/api/organisations/tasks.py +++ b/api/organisations/tasks.py @@ -14,7 +14,7 @@ from app_analytics.analytics_db_service import get_total_events_count from app_analytics.influxdb_wrapper import get_current_api_usage -from integrations.flagsmith.client import get_client +from integrations.flagsmith.client import get_openfeature_client from organisations import subscription_info_cache from organisations.chargebee import ( # type: ignore[attr-defined] add_100k_api_calls_scale_up, @@ -113,7 +113,7 @@ def finish_subscription_cancellation() -> None: # Task enqueued in register_recurring_tasks below. def handle_api_usage_notifications() -> None: - flagsmith_client = get_client("local", local_eval=True) + openfeature_client = get_openfeature_client() threshold_percentage = min(API_USAGE_ALERT_THRESHOLDS) / 100 for organisation in Organisation.objects.filter( @@ -129,10 +129,11 @@ def handle_api_usage_notifications() -> None: ) * threshold_percentage, ).select_related("subscription", "subscription_information_cache"): - feature_enabled = flagsmith_client.get_identity_flags( - organisation.flagsmith_identifier, - traits=organisation.flagsmith_on_flagsmith_api_traits, - ).is_feature_enabled("api_usage_alerting") + feature_enabled = openfeature_client.get_boolean_value( + "api_usage_alerting", + default_value=False, + evaluation_context=organisation.openfeature_evaluation_context, + ) if not feature_enabled: logger.info( f"Skipping processing API usage for organisation {organisation.id}" @@ -174,7 +175,7 @@ def charge_for_api_call_count_overages(): # type: ignore[no-untyped-def] ).values_list("organisation_id", flat=True) ) - flagsmith_client = get_client("local", local_eval=True) + openfeature_client = get_openfeature_client() for organisation in ( Organisation.objects.filter( @@ -199,17 +200,19 @@ def charge_for_api_call_count_overages(): # type: ignore[no-untyped-def] "subscription", ) ): - flags = flagsmith_client.get_identity_flags( - organisation.flagsmith_identifier, - traits=organisation.flagsmith_on_flagsmith_api_traits, - ) - if not flags.is_feature_enabled("api_usage_overage_charges"): + if not openfeature_client.get_boolean_value( + "api_usage_overage_charges", + default_value=False, + evaluation_context=organisation.openfeature_evaluation_context, + ): continue subscription_cache = organisation.subscription_information_cache # TODO: Default to get_total_events_count — https://github.com/Flagsmith/flagsmith/issues/6985 - if flags.is_feature_enabled( - "get_current_api_usage_deprecated" + if openfeature_client.get_boolean_value( + "get_current_api_usage_deprecated", + default_value=False, + evaluation_context=organisation.openfeature_evaluation_context, ): # pragma: no cover api_usage = get_total_events_count( organisation, @@ -329,16 +332,21 @@ def restrict_use_due_to_api_limit_grace_period_over() -> None: ) api_limit_access_blocks = [] - flagsmith_client = get_client("local", local_eval=True) + openfeature_client = get_openfeature_client() for organisation in organisations: - flags = flagsmith_client.get_identity_flags( - organisation.flagsmith_identifier, - traits=organisation.flagsmith_on_flagsmith_api_traits, - ) + ctx = organisation.openfeature_evaluation_context - stop_serving = flags.is_feature_enabled("api_limiting_stop_serving_flags") - block_access = flags.is_feature_enabled("api_limiting_block_access_to_admin") + stop_serving = openfeature_client.get_boolean_value( + "api_limiting_stop_serving_flags", + default_value=False, + evaluation_context=ctx, + ) + block_access = openfeature_client.get_boolean_value( + "api_limiting_block_access_to_admin", + default_value=False, + evaluation_context=ctx, + ) if not stop_serving and not block_access: continue @@ -350,8 +358,10 @@ def restrict_use_due_to_api_limit_grace_period_over() -> None: subscription_cache = organisation.subscription_information_cache # TODO: Default to get_total_events_count — https://github.com/Flagsmith/flagsmith/issues/6985 - if flags.is_feature_enabled( - "get_current_api_usage_deprecated" + if openfeature_client.get_boolean_value( + "get_current_api_usage_deprecated", + default_value=False, + evaluation_context=ctx, ): # pragma: no cover api_usage = get_total_events_count( organisation, diff --git a/api/organisations/urls.py b/api/organisations/urls.py index 95ac0febdc23..05a0ed976096 100644 --- a/api/organisations/urls.py +++ b/api/organisations/urls.py @@ -161,7 +161,7 @@ ] if settings.LICENSING_INSTALLED: # pragma: no cover - from licensing.views import ( # type: ignore[import-not-found,import-untyped,unused-ignore] + from licensing.views import ( # type: ignore[import-not-found,unused-ignore] create_or_update_licence, ) diff --git a/api/poetry.lock b/api/poetry.lock index abebe6950a72..3793de56a5ba 100644 --- a/api/poetry.lock +++ b/api/poetry.lock @@ -3126,6 +3126,33 @@ rsa = ["cryptography (>=3.0.0)"] signals = ["blinker (>=1.4.0)"] signedtoken = ["cryptography (>=3.0.0)", "pyjwt (>=2.0.0,<3)"] +[[package]] +name = "openfeature-provider-flagsmith" +version = "0.1.6" +description = "Openfeature provider for Flagsmith" +optional = false +python-versions = "<4.0,>=3.9" +groups = ["main"] +files = [ + {file = "openfeature_provider_flagsmith-0.1.6.tar.gz", hash = "sha256:49fe9f779dd83f3e73ce1480dd16e8a2cd332e237e48a4d08e200282a4c3f206"}, +] + +[package.dependencies] +flagsmith = ">=3.6.0,<6.0.0" +openfeature-sdk = ">=0.6.0,<0.9.0" + +[[package]] +name = "openfeature-sdk" +version = "0.8.4" +description = "Standardizing Feature Flagging for Everyone" +optional = false +python-versions = ">=3.9" +groups = ["main"] +files = [ + {file = "openfeature_sdk-0.8.4-py3-none-any.whl", hash = "sha256:805ba090669798fc343ca9fdcbc56ff0f4b57bf6757533f0854d2021192e620a"}, + {file = "openfeature_sdk-0.8.4.tar.gz", hash = "sha256:66abf71f928ec8c0db1111072bb0ef2635dfbd09510f77f4b548e5d0ea0e6c1a"}, +] + [[package]] name = "packaging" version = "25.0" @@ -5671,4 +5698,4 @@ files = [ [metadata] lock-version = "2.1" python-versions = ">3.11,<3.14" -content-hash = "be68d9e58505506bea67c09d1d96dcf5ba4ce14ee09e6fe7f8d43fbfb6ff9353" +content-hash = "5316413c16f297365a20c9d94de76ed86a205765c43f6e7e88c607d8d7da3caa" diff --git a/api/pyproject.toml b/api/pyproject.toml index 86c1dd2c667d..3e6993814bdb 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -94,6 +94,14 @@ ignore_missing_imports = true module = ["saml.*"] ignore_missing_imports = true +[[tool.mypy.overrides]] +module = ["licensing.*"] +ignore_missing_imports = true + +[[tool.mypy.overrides]] +module = ["openfeature_flagsmith.*"] +ignore_missing_imports = true + [tool.django-stubs] django_settings_module = "app.settings.local" @@ -157,6 +165,8 @@ pydantic = "^2.12.0" pydantic-collections = "^0.6.0" pyngo = "~2.4.1" flagsmith = "^5.1.1" +openfeature-sdk = ">=0.6.0,<0.9.0" +openfeature-provider-flagsmith = ">=0.1.6" python-gnupg = "^0.5.1" django-redis = "^5.4.0" pygithub = "~2.8" @@ -221,7 +231,7 @@ pytest-lazy-fixture = "~0.6.3" moto = "~4.1.3" # TODO: move to https://github.com/adamchainz/time-machine pytest-freezegun = "^0.4.2" -setuptools = "*" # this only exists because pytest-freezegun fails without it +setuptools = "*" # this only exists because pytest-freezegun fails without it pytest-xdist = "~3.6.1" pylint = "~2.16.2" pep8 = "~1.7.1" diff --git a/api/tests/unit/integrations/flagsmith/test_unit_flagsmith_client.py b/api/tests/unit/integrations/flagsmith/test_unit_flagsmith_client.py index 866ed3af56fd..7579619267b6 100644 --- a/api/tests/unit/integrations/flagsmith/test_unit_flagsmith_client.py +++ b/api/tests/unit/integrations/flagsmith/test_unit_flagsmith_client.py @@ -2,40 +2,117 @@ import pytest from flagsmith.offline_handlers import LocalFileHandler +from openfeature.provider import ProviderStatus from pytest_django.fixtures import SettingsWrapper from pytest_mock import MockerFixture -from integrations.flagsmith.client import get_client +from integrations.flagsmith.client import ( + DEFAULT_OPENFEATURE_DOMAIN, + get_openfeature_client, + get_provider_kwargs, + initialise_provider, +) from integrations.flagsmith.exceptions import FlagsmithIntegrationError from integrations.flagsmith.flagsmith_service import ENVIRONMENT_JSON_PATH -@pytest.fixture(autouse=True) -def reset_globals(mocker: MockerFixture) -> None: # type: ignore[misc] - mocker.patch("integrations.flagsmith.client._flagsmith_clients", {}) - yield - - @pytest.fixture() -def mock_local_file_handler(mocker: MockerFixture) -> None: - return mocker.MagicMock(spec=LocalFileHandler) # type: ignore[no-any-return] +def mock_local_file_handler(mocker: MockerFixture) -> MagicMock: + mock: MagicMock = mocker.MagicMock(spec=LocalFileHandler) + return mock @pytest.fixture() -def mock_local_file_handler_class( # type: ignore[no-untyped-def] +def mock_local_file_handler_class( mocker: MockerFixture, mock_local_file_handler: MagicMock -): +) -> MagicMock: return mocker.patch( "integrations.flagsmith.client.LocalFileHandler", return_value=mock_local_file_handler, ) -def test_get_client__offline_mode_disabled__initialises_with_server_key( # type: ignore[no-untyped-def] +def test_get_openfeature_client__provider_not_ready__initialises_provider( settings: SettingsWrapper, mocker: MockerFixture, - mock_local_file_handler, - mock_local_file_handler_class, + mock_local_file_handler: MagicMock, + mock_local_file_handler_class: MagicMock, +) -> None: + # Given + settings.FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE = True + + mock_openfeature_api = mocker.patch("integrations.flagsmith.client.openfeature_api") + mock_client = mock_openfeature_api.get_client.return_value + mock_client.get_provider_status.return_value = ProviderStatus.NOT_READY + + mock_flagsmith_class = mocker.patch("integrations.flagsmith.client.Flagsmith") + mock_provider_class = mocker.patch( + "integrations.flagsmith.client.FlagsmithProvider" + ) + + # When + client = get_openfeature_client() + + # Then + assert client == mock_client + mock_flagsmith_class.assert_called_once() + mock_provider_class.assert_called_once_with( + client=mock_flagsmith_class.return_value, + ) + mock_openfeature_api.set_provider.assert_called_once_with( + mock_provider_class.return_value, + domain=DEFAULT_OPENFEATURE_DOMAIN, + ) + + +def test_get_openfeature_client__provider_ready__skips_initialisation( + mocker: MockerFixture, +) -> None: + # Given + mock_openfeature_api = mocker.patch("integrations.flagsmith.client.openfeature_api") + mock_client = mock_openfeature_api.get_client.return_value + mock_client.get_provider_status.return_value = ProviderStatus.READY + + mock_flagsmith_class = mocker.patch("integrations.flagsmith.client.Flagsmith") + + # When + client = get_openfeature_client() + + # Then + assert client == mock_client + mock_flagsmith_class.assert_not_called() + mock_openfeature_api.set_provider.assert_not_called() + + +def test_get_openfeature_client__custom_flagsmith_kwargs__passes_to_provider( + settings: SettingsWrapper, + mocker: MockerFixture, + mock_local_file_handler: MagicMock, + mock_local_file_handler_class: MagicMock, +) -> None: + # Given + settings.FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE = True + + mock_openfeature_api = mocker.patch("integrations.flagsmith.client.openfeature_api") + mock_client = mock_openfeature_api.get_client.return_value + mock_client.get_provider_status.return_value = ProviderStatus.NOT_READY + + mock_flagsmith_class = mocker.patch("integrations.flagsmith.client.Flagsmith") + mocker.patch("integrations.flagsmith.client.FlagsmithProvider") + + # When + get_openfeature_client(enable_local_evaluation=False) + + # Then + call_args = mock_flagsmith_class.call_args + assert call_args.kwargs["enable_local_evaluation"] is False + + +def test_initialise_provider__offline_mode_disabled__initialises_with_server_key( + settings: SettingsWrapper, + mocker: MockerFixture, + mock_local_file_handler: MagicMock, + mock_local_file_handler_class: MagicMock, ) -> None: # Given server_key = "some-key" @@ -46,57 +123,77 @@ def test_get_client__offline_mode_disabled__initialises_with_server_key( # type settings.FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE = False mock_flagsmith_class = mocker.patch("integrations.flagsmith.client.Flagsmith") + mock_provider_class = mocker.patch( + "integrations.flagsmith.client.FlagsmithProvider" + ) + mock_openfeature_api = mocker.patch("integrations.flagsmith.client.openfeature_api") # When - client = get_client() + initialise_provider(**get_provider_kwargs()) # Then - assert client == mock_flagsmith_class.return_value - mock_flagsmith_class.assert_called_once() - call_args = mock_flagsmith_class.call_args assert call_args.kwargs["environment_key"] == server_key assert call_args.kwargs["api_url"] == api_url + assert call_args.kwargs["enable_local_evaluation"] is True assert "offline_mode" not in call_args.kwargs assert call_args.kwargs["offline_handler"] == mock_local_file_handler + mock_provider_class.assert_called_once_with( + client=mock_flagsmith_class.return_value, + ) + mock_openfeature_api.set_provider.assert_called_once_with( + mock_provider_class.return_value, + domain=DEFAULT_OPENFEATURE_DOMAIN, + ) + mock_local_file_handler_class.assert_called_once_with(ENVIRONMENT_JSON_PATH) -def test_get_client__offline_mode_enabled__initialises_with_offline_handler( # type: ignore[no-untyped-def] +def test_initialise_provider__offline_mode_enabled__initialises_with_offline_handler( settings: SettingsWrapper, mocker: MockerFixture, - mock_local_file_handler, - mock_local_file_handler_class, + mock_local_file_handler: MagicMock, + mock_local_file_handler_class: MagicMock, ) -> None: # Given settings.FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE = True mock_flagsmith_class = mocker.patch("integrations.flagsmith.client.Flagsmith") + mock_provider_class = mocker.patch( + "integrations.flagsmith.client.FlagsmithProvider" + ) + mock_openfeature_api = mocker.patch("integrations.flagsmith.client.openfeature_api") # When - client = get_client() + initialise_provider(**get_provider_kwargs()) # Then - assert client == mock_flagsmith_class.return_value - mock_flagsmith_class.assert_called_once() - call_args = mock_flagsmith_class.call_args assert call_args.kwargs["offline_mode"] is True + assert call_args.kwargs["enable_local_evaluation"] is True assert call_args.kwargs["offline_handler"] == mock_local_file_handler + mock_provider_class.assert_called_once_with( + client=mock_flagsmith_class.return_value, + ) + mock_openfeature_api.set_provider.assert_called_once_with( + mock_provider_class.return_value, + domain=DEFAULT_OPENFEATURE_DOMAIN, + ) + mock_local_file_handler_class.assert_called_once_with(ENVIRONMENT_JSON_PATH) -def test_get_client__missing_server_key__raises_error( # type: ignore[no-untyped-def] - settings: SettingsWrapper, mock_local_file_handler_class -): +def test_get_provider_kwargs__missing_server_key__raises_error( + settings: SettingsWrapper, mock_local_file_handler_class: MagicMock +) -> None: # Given settings.FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE = False assert settings.FLAGSMITH_ON_FLAGSMITH_SERVER_KEY is None # When / Then with pytest.raises(FlagsmithIntegrationError): - get_client() + get_provider_kwargs() diff --git a/api/tests/unit/organisations/test_unit_organisations_tasks.py b/api/tests/unit/organisations/test_unit_organisations_tasks.py index e6414edb401a..8916f70dcb83 100644 --- a/api/tests/unit/organisations/test_unit_organisations_tasks.py +++ b/api/tests/unit/organisations/test_unit_organisations_tasks.py @@ -408,10 +408,6 @@ def test_handle_api_usage_notifications__feature_flag_is_off__skips_processing( mock_api_usage = mocker.patch( "organisations.tasks.get_current_api_usage", ) - get_client_mock = mocker.patch("organisations.tasks.get_client") - client_mock = MagicMock() - get_client_mock.return_value = client_mock - client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = False # When handle_api_usage_notifications() @@ -419,15 +415,6 @@ def test_handle_api_usage_notifications__feature_flag_is_off__skips_processing( # Then mock_api_usage.assert_not_called() - client_mock.get_identity_flags.assert_called_once_with( - organisation.flagsmith_identifier, - traits={ - "organisation.id": organisation.id, - "organisation.name": organisation.name, - "subscription.plan": organisation.subscription.plan, - }, - ) - assert len(mailoutbox) == 0 assert ( OrganisationAPIUsageNotification.objects.filter( @@ -442,6 +429,7 @@ def test_handle_api_usage_notifications__usage_below_100_percent__sends_90_perce mocker: MockerFixture, organisation: Organisation, mailoutbox: list[EmailMultiAlternatives], + enable_features: EnableFeaturesFixture, ) -> None: # Given now = timezone.now() @@ -459,10 +447,7 @@ def test_handle_api_usage_notifications__usage_below_100_percent__sends_90_perce "organisations.task_helpers.get_current_api_usage", ) mock_api_usage.return_value = 91 - get_client_mock = mocker.patch("organisations.tasks.get_client") - client_mock = MagicMock() - get_client_mock.return_value = client_mock - client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = True + enable_features("api_usage_alerting") assert not OrganisationAPIUsageNotification.objects.filter( organisation=organisation, @@ -554,6 +539,7 @@ def test_handle_api_usage_notifications__usage_below_alert_thresholds__sends_no_ mocker: MockerFixture, organisation: Organisation, mailoutbox: list[EmailMultiAlternatives], + enable_features: EnableFeaturesFixture, ) -> None: # Given now = timezone.now() @@ -576,10 +562,7 @@ def test_handle_api_usage_notifications__usage_below_alert_thresholds__sends_no_ usage = 21 assert usage < min(API_USAGE_ALERT_THRESHOLDS) mock_api_usage.return_value = usage - get_client_mock = mocker.patch("organisations.tasks.get_client") - client_mock = MagicMock() - get_client_mock.return_value = client_mock - client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = True + enable_features("api_usage_alerting") assert not OrganisationAPIUsageNotification.objects.filter( organisation=organisation, @@ -604,6 +587,7 @@ def test_handle_api_usage_notifications__usage_above_100_percent__sends_limit_no mocker: MockerFixture, organisation: Organisation, mailoutbox: list[EmailMultiAlternatives], + enable_features: EnableFeaturesFixture, ) -> None: # Given usage = 105 @@ -625,11 +609,7 @@ def test_handle_api_usage_notifications__usage_above_100_percent__sends_limit_no "organisations.task_helpers.get_current_api_usage", ) mock_api_usage.return_value = usage - - get_client_mock = mocker.patch("organisations.tasks.get_client") - client_mock = MagicMock() - get_client_mock.return_value = client_mock - client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = True + enable_features("api_usage_alerting") assert not OrganisationAPIUsageNotification.objects.filter( organisation=organisation, @@ -718,10 +698,10 @@ def test_handle_api_usage_notifications__processing_error__logs_error_message( api_calls_30d=100, ) - get_client_mock = mocker.patch("organisations.tasks.get_client") + get_client_mock = mocker.patch("organisations.tasks.get_openfeature_client") client_mock = MagicMock() get_client_mock.return_value = client_mock - client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = True + client_mock.get_boolean_value.return_value = True api_usage_patch = mocker.patch( "organisations.tasks.handle_api_usage_notification_for_organisation", @@ -753,6 +733,7 @@ def test_handle_api_usage_notifications__free_account_over_limit__sends_limit_no mocker: MockerFixture, organisation: Organisation, mailoutbox: list[EmailMultiAlternatives], + enable_features: EnableFeaturesFixture, ) -> None: # Given allowance = MAX_SEATS_IN_FREE_PLAN @@ -773,11 +754,7 @@ def test_handle_api_usage_notifications__free_account_over_limit__sends_limit_no "organisations.task_helpers.get_current_api_usage", ) mock_api_usage.return_value = usage - - get_client_mock = mocker.patch("organisations.tasks.get_client") - client_mock = MagicMock() - get_client_mock.return_value = client_mock - client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = True + enable_features("api_usage_alerting") assert not OrganisationAPIUsageNotification.objects.filter( organisation=organisation, @@ -852,6 +829,7 @@ def test_handle_api_usage_notifications__missing_info_cache__skips_processing( organisation: Organisation, mailoutbox: list[EmailMultiAlternatives], inspecting_handler: logging.Handler, + enable_features: EnableFeaturesFixture, ) -> None: # Given organisation.subscription.plan = SCALE_UP @@ -862,11 +840,7 @@ def test_handle_api_usage_notifications__missing_info_cache__skips_processing( mock_api_usage = mocker.patch( "organisations.task_helpers.get_current_api_usage", ) - - get_client_mock = mocker.patch("organisations.tasks.get_client") - client_mock = MagicMock() - get_client_mock.return_value = client_mock - client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = True + enable_features("api_usage_alerting") assert not OrganisationAPIUsageNotification.objects.filter( organisation=organisation, @@ -929,7 +903,6 @@ def test_charge_for_api_call_count_overages__scale_up_plan__charges_correct_addo "organisations.chargebee.chargebee.chargebee_client.Subscription.update", autospec=True, ) - enable_features("api_usage_overage_charges") mock_api_usage = mocker.patch( @@ -990,10 +963,6 @@ def test_charge_for_api_call_count_overages__cancellation_date_set__skips_chargi notified_at=now, ) - get_client_mock = mocker.patch("organisations.tasks.get_client") - client_mock = MagicMock() - get_client_mock.return_value = client_mock - mock_api_usage = mocker.patch( "organisations.tasks.get_current_api_usage", ) @@ -1005,7 +974,6 @@ def test_charge_for_api_call_count_overages__cancellation_date_set__skips_chargi # Then assert OrganisationAPIBilling.objects.count() == 0 mock_api_usage.assert_not_called() - client_mock.get_identity_flags.assert_not_called() @pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00") @@ -1042,11 +1010,6 @@ def test_charge_for_api_call_count_overages__flagsmith_feature_disabled__skips_c autospec=True, ) - get_client_mock = mocker.patch("organisations.tasks.get_client") - client_mock = MagicMock() - get_client_mock.return_value = client_mock - client_mock.get_identity_flags.return_value.is_feature_enabled.return_value = False - mock_api_usage = mocker.patch( "organisations.tasks.get_current_api_usage", ) @@ -1058,14 +1021,6 @@ def test_charge_for_api_call_count_overages__flagsmith_feature_disabled__skips_c # Then # No charges are applied to the account. - client_mock.get_identity_flags.assert_called_once_with( - organisation.flagsmith_identifier, - traits={ - "organisation.id": organisation.id, - "organisation.name": organisation.name, - "subscription.plan": organisation.subscription.plan, - }, - ) mock_chargebee_update.assert_not_called() assert OrganisationAPIBilling.objects.count() == 0 @@ -1935,7 +1890,6 @@ def test_restrict_use_due_to_api_limit_grace_period_over__previously_breached__b @pytest.mark.freeze_time("2023-01-19T09:09:47.325132+00:00") def test_restrict_use_due_to_api_limit_grace_period_over__missing_subscription_cache__does_not_block( - mocker: MockerFixture, organisation: Organisation, freezer: FrozenDateTimeFactory, mailoutbox: list[EmailMultiAlternatives], @@ -1943,7 +1897,6 @@ def test_restrict_use_due_to_api_limit_grace_period_over__missing_subscription_c ) -> None: # Given assert not organisation.has_subscription_information_cache() - enable_features( "api_limiting_stop_serving_flags", "api_limiting_block_access_to_admin", @@ -1991,7 +1944,6 @@ def test_restrict_use_due_to_api_limit_grace_period_over__reduced_api_usage__doe from organisations.tasks import logger logger.addHandler(inspecting_handler) - enable_features( "api_limiting_stop_serving_flags", "api_limiting_block_access_to_admin",