diff --git a/api/conftest.py b/api/conftest.py index deec0b19fb03..c84fed83b7bd 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -1259,10 +1259,28 @@ def set_github_webhook_secret() -> None: settings.GITHUB_WEBHOOK_SECRET = "secret-key" +@pytest.fixture(autouse=True) +def _openfeature_no_flags_by_default() -> typing.Generator[None, None, None]: + """ + Bind an empty `InMemoryProvider` to the Flagsmith domain for every test. + + Without this, `get_openfeature_client` would fall through to a real + `FlagsmithProvider` reading the bundled environment JSON, leaking + production-shaped flag state into unit tests. Tests that need flags on + should use the `enable_features` fixture. + """ + openfeature_api.set_provider( + InMemoryProvider({}), + domain=DEFAULT_OPENFEATURE_DOMAIN, + ) + yield + openfeature_api.clear_providers() + + @pytest.fixture() def enable_features( mocker: MockerFixture, -) -> typing.Generator[EnableFeaturesFixture, None, None]: +) -> EnableFeaturesFixture: """ This fixture returns a callable that allows us to enable any Flagsmith feature flag(s) in tests. @@ -1283,9 +1301,7 @@ def _enable_features(*expected_feature_names: str) -> None: domain=DEFAULT_OPENFEATURE_DOMAIN, ) - yield _enable_features - - openfeature_api.clear_providers() + return _enable_features @pytest.fixture(autouse=True) diff --git a/api/integrations/flagsmith/client.py b/api/integrations/flagsmith/client.py index b518df262fc2..4ada0eb41d4a 100644 --- a/api/integrations/flagsmith/client.py +++ b/api/integrations/flagsmith/client.py @@ -20,7 +20,6 @@ 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 @@ -33,7 +32,10 @@ def get_openfeature_client( domain: str = DEFAULT_OPENFEATURE_DOMAIN, ) -> OpenFeatureClient: openfeature_client = openfeature_api.get_client(domain=domain) - if openfeature_client.get_provider_status() != ProviderStatus.READY: + # An unbound domain falls back to a ready `NoOpProvider`, so we can't rely + # on provider status here — check whether we're still on the default. + metadata = openfeature_api.get_provider_metadata(domain) + if getattr(metadata, "is_default_provider", False): initialise_provider(domain, **get_provider_kwargs()) return openfeature_client 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 dbc446faf34e..8af126f9c9cf 100644 --- a/api/tests/unit/integrations/flagsmith/test_unit_flagsmith_client.py +++ b/api/tests/unit/integrations/flagsmith/test_unit_flagsmith_client.py @@ -1,8 +1,12 @@ from unittest.mock import MagicMock +import openfeature.api as openfeature_api import pytest from flagsmith.offline_handlers import LocalFileHandler -from openfeature.provider import ProviderStatus +from openfeature.provider.in_memory_provider import InMemoryProvider +from openfeature.provider.metadata import Metadata +from openfeature.provider.no_op_metadata import NoOpMetadata +from openfeature_flagsmith.provider import FlagsmithProvider from pytest_django.fixtures import SettingsWrapper from pytest_mock import MockerFixture @@ -32,7 +36,7 @@ def mock_local_file_handler_class( ) -def test_get_openfeature_client__provider_not_ready__initialises_provider( +def test_get_openfeature_client__default_provider__initialises_provider( settings: SettingsWrapper, mocker: MockerFixture, mock_local_file_handler: MagicMock, @@ -42,13 +46,10 @@ def test_get_openfeature_client__provider_not_ready__initialises_provider( settings.FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE = True mock_openfeature_api = mocker.patch("integrations.flagsmith.client.openfeature_api") + mock_openfeature_api.get_provider_metadata.return_value = NoOpMetadata() 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() @@ -56,22 +57,26 @@ def test_get_openfeature_client__provider_not_ready__initialises_provider( # 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, - ) + mock_openfeature_api.set_provider.assert_called_once() + (provider_arg,) = mock_openfeature_api.set_provider.call_args.args + assert isinstance(provider_arg, FlagsmithProvider) + assert mock_openfeature_api.set_provider.call_args.kwargs == { + "domain": DEFAULT_OPENFEATURE_DOMAIN, + } -def test_get_openfeature_client__provider_ready__skips_initialisation( +def test_get_openfeature_client__non_default_provider__skips_initialisation( mocker: MockerFixture, ) -> None: # Given + # Any provider explicitly bound to the domain — including test doubles + # like `InMemoryProvider` from the `enable_features` fixture — must be + # preserved by `get_openfeature_client`. mock_openfeature_api = mocker.patch("integrations.flagsmith.client.openfeature_api") + mock_openfeature_api.get_provider_metadata.return_value = Metadata( + name="some-other-provider", + ) 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") @@ -84,6 +89,58 @@ def test_get_openfeature_client__provider_ready__skips_initialisation( mock_openfeature_api.set_provider.assert_not_called() +def test_get_openfeature_client__first_call__installs_flagsmith_provider_for_domain( + settings: SettingsWrapper, + mocker: MockerFixture, + mock_local_file_handler_class: MagicMock, +) -> None: + # Given + # `openfeature_api` is intentionally NOT mocked so the real provider + # registry is exercised: an uninitialised domain falls back to a ready + # `NoOpProvider`, so a naive `status != READY` guard never triggers + # initialisation and every evaluation silently returns the default. + settings.FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE = True + mocker.patch("integrations.flagsmith.client.Flagsmith") + openfeature_api.clear_providers() + + # When + get_openfeature_client() + + # Then + installed_provider = openfeature_api.get_client( + domain=DEFAULT_OPENFEATURE_DOMAIN + ).provider + assert isinstance(installed_provider, FlagsmithProvider) + + # Cleanup + openfeature_api.clear_providers() + + +def test_get_openfeature_client__externally_installed_provider__preserved( + mocker: MockerFixture, +) -> None: + # Given + # Exercised against the real `openfeature_api` to confirm the + # `enable_features` fixture's `InMemoryProvider` survives a call to + # `get_openfeature_client`. + mocker.patch("integrations.flagsmith.client.Flagsmith") + openfeature_api.clear_providers() + in_memory_provider = InMemoryProvider({}) + openfeature_api.set_provider(in_memory_provider, domain=DEFAULT_OPENFEATURE_DOMAIN) + + # When + get_openfeature_client() + + # Then + installed_provider = openfeature_api.get_client( + domain=DEFAULT_OPENFEATURE_DOMAIN + ).provider + assert installed_provider is in_memory_provider + + # Cleanup + openfeature_api.clear_providers() + + def test_initialise_provider__offline_mode_disabled__initialises_with_server_key( settings: SettingsWrapper, mocker: MockerFixture,