From ed6a42f264a6efc149642181bfc4c6de0eb83ce1 Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Tue, 20 Feb 2024 19:31:22 -0300 Subject: [PATCH] feat!: add support for domains (#271) * feat: add support for domains Signed-off-by: Federico Bond * docs: update README.md Signed-off-by: Federico Bond * feat: add clear_providers function to api Signed-off-by: Federico Bond * feat: make _get_provider function private Signed-off-by: Federico Bond * fix: shutdown all providers on api.shutdown Signed-off-by: Federico Bond * refactor: move provider dict to a ProviderRegistry class Signed-off-by: Federico Bond * feat: reset default provider on clear_providers and add tests Signed-off-by: Federico Bond * docs: update README.md Signed-off-by: Federico Bond --------- Signed-off-by: Federico Bond --- README.md | 29 ++++++-- openfeature/api.py | 37 +++++------ openfeature/client.py | 15 +++-- openfeature/provider/registry.py | 59 +++++++++++++++++ tests/features/steps/steps.py | 4 +- tests/test_api.py | 110 +++++++++++++++++++++++++------ tests/test_client.py | 10 +-- 7 files changed, 208 insertions(+), 56 deletions(-) create mode 100644 openfeature/provider/registry.py diff --git a/README.md b/README.md index 1cfae8e5..9c813390 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ print("Value: " + str(flag_value)) | ✅ | [Targeting](#targeting) | Contextually-aware flag evaluation using [evaluation context](https://openfeature.dev/docs/reference/concepts/evaluation-context). | | ✅ | [Hooks](#hooks) | Add functionality to various stages of the flag evaluation life-cycle. | | ✅ | [Logging](#logging) | Integrate with popular logging packages. | -| ❌ | [Named clients](#named-clients) | Utilize multiple providers in a single application. | +| ✅ | [Domains](#domains) | Logically bind clients with providers. | | ❌ | [Eventing](#eventing) | React to state changes in the provider or flag management system. | | ✅ | [Shutdown](#shutdown) | Gracefully clean up a provider during application shutdown. | | ✅ | [Extending](#extending) | Extend OpenFeature with custom providers and hooks. | @@ -128,8 +128,8 @@ api.set_provider(NoOpProvider()) open_feature_client = api.get_client() ``` - +In some situations, it may be beneficial to register multiple providers in the same application. +This is possible using [domains](#domains), which is covered in more detail below. ### Targeting @@ -189,9 +189,28 @@ client.get_boolean_flag("my-flag", False, flag_evaluation_options=options) The OpenFeature SDK logs to the `openfeature` logger using the `logging` package from the Python Standard Library. -### Named clients +### Domains -Named clients are not yet available in the Python SDK. Progress on this feature can be tracked [here](https://github.com/open-feature/python-sdk/issues/125). +Clients can be assigned to a domain. +A domain is a logical identifier which can be used to associate clients with a particular provider. +If a domain has no associated provider, the global provider is used. + +```python +from openfeature import api + +# Registering the default provider +api.set_provider(MyProvider()); +# Registering a provider to a domain +api.set_provider(MyProvider(), "my-domain"); + +# A client bound to the default provider +default_client = api.get_client(); +# A client bound to the MyProvider provider +domain_scoped_client = api.get_client("my-domain"); +``` + +Domains can be defined on a provider during registration. +For more details, please refer to the [providers](#providers) section. ### Eventing diff --git a/openfeature/api.py b/openfeature/api.py index 8ce1945b..f4574545 100644 --- a/openfeature/api.py +++ b/openfeature/api.py @@ -6,39 +6,36 @@ from openfeature.hook import Hook from openfeature.provider import FeatureProvider from openfeature.provider.metadata import Metadata -from openfeature.provider.no_op_provider import NoOpProvider - -_provider: FeatureProvider = NoOpProvider() +from openfeature.provider.registry import ProviderRegistry _evaluation_context = EvaluationContext() _hooks: typing.List[Hook] = [] +_provider_registry: ProviderRegistry = ProviderRegistry() + def get_client( - name: typing.Optional[str] = None, version: typing.Optional[str] = None + domain: typing.Optional[str] = None, version: typing.Optional[str] = None ) -> OpenFeatureClient: - return OpenFeatureClient(name=name, version=version, provider=_provider) + return OpenFeatureClient(domain=domain, version=version) -def set_provider(provider: FeatureProvider) -> None: - global _provider - if provider is None: - raise GeneralError(error_message="No provider") - if _provider: - _provider.shutdown() - _provider = provider - provider.initialize(_evaluation_context) +def set_provider( + provider: FeatureProvider, domain: typing.Optional[str] = None +) -> None: + if domain is None: + _provider_registry.set_default_provider(provider) + else: + _provider_registry.set_provider(domain, provider) -def get_provider() -> FeatureProvider: - global _provider - return _provider +def clear_providers() -> None: + return _provider_registry.clear_providers() -def get_provider_metadata() -> Metadata: - global _provider - return _provider.get_metadata() +def get_provider_metadata(domain: typing.Optional[str] = None) -> Metadata: + return _provider_registry.get_provider(domain).get_metadata() def get_evaluation_context() -> EvaluationContext: @@ -69,4 +66,4 @@ def get_hooks() -> typing.List[Hook]: def shutdown() -> None: - _provider.shutdown() + _provider_registry.shutdown() diff --git a/openfeature/client.py b/openfeature/client.py index 092290f8..deac93c8 100644 --- a/openfeature/client.py +++ b/openfeature/client.py @@ -60,26 +60,29 @@ @dataclass class ClientMetadata: - name: typing.Optional[str] + name: typing.Optional[str] = None + domain: typing.Optional[str] = None class OpenFeatureClient: def __init__( self, - name: typing.Optional[str], + domain: typing.Optional[str], version: typing.Optional[str], - provider: FeatureProvider, context: typing.Optional[EvaluationContext] = None, hooks: typing.Optional[typing.List[Hook]] = None, ) -> None: - self.name = name + self.domain = domain self.version = version self.context = context or EvaluationContext() self.hooks = hooks or [] - self.provider = provider + + @property + def provider(self) -> FeatureProvider: + return api._provider_registry.get_provider(self.domain) def get_metadata(self) -> ClientMetadata: - return ClientMetadata(name=self.name) + return ClientMetadata(domain=self.domain) def add_hooks(self, hooks: typing.List[Hook]) -> None: self.hooks = self.hooks + hooks diff --git a/openfeature/provider/registry.py b/openfeature/provider/registry.py new file mode 100644 index 00000000..55b59931 --- /dev/null +++ b/openfeature/provider/registry.py @@ -0,0 +1,59 @@ +import typing + +from openfeature.evaluation_context import EvaluationContext +from openfeature.exception import GeneralError +from openfeature.provider import FeatureProvider +from openfeature.provider.no_op_provider import NoOpProvider + + +class ProviderRegistry: + _default_provider: FeatureProvider + _providers: typing.Dict[str, FeatureProvider] + + def __init__(self) -> None: + self._default_provider = NoOpProvider() + self._providers = {} + + def set_provider(self, domain: str, provider: FeatureProvider) -> None: + if provider is None: + raise GeneralError(error_message="No provider") + providers = self._providers + if domain in providers: + old_provider = providers[domain] + del providers[domain] + if old_provider not in providers.values(): + old_provider.shutdown() + if provider not in providers.values(): + provider.initialize(self._get_evaluation_context()) + providers[domain] = provider + + def get_provider(self, domain: typing.Optional[str]) -> FeatureProvider: + if domain is None: + return self._default_provider + return self._providers.get(domain, self._default_provider) + + def set_default_provider(self, provider: FeatureProvider) -> None: + if provider is None: + raise GeneralError(error_message="No provider") + if self._default_provider: + self._default_provider.shutdown() + self._default_provider = provider + provider.initialize(self._get_evaluation_context()) + + def get_default_provider(self) -> FeatureProvider: + return self._default_provider + + def clear_providers(self) -> None: + self.shutdown() + self._providers.clear() + self._default_provider = NoOpProvider() + + def shutdown(self) -> None: + for provider in {self._default_provider, *self._providers.values()}: + provider.shutdown() + + def _get_evaluation_context(self) -> EvaluationContext: + # imported here to avoid circular imports + from openfeature.api import get_evaluation_context + + return get_evaluation_context() diff --git a/tests/features/steps/steps.py b/tests/features/steps/steps.py index e20687fc..ff517dfa 100644 --- a/tests/features/steps/steps.py +++ b/tests/features/steps/steps.py @@ -27,7 +27,7 @@ def step_impl(context, flag_type, key, expected_reason): @given("a provider is registered with cache disabled") def step_impl(context): set_provider(InMemoryProvider(IN_MEMORY_FLAGS)) - context.client = get_client(name="Default Provider", version="1.0") + context.client = get_client() @when( @@ -35,7 +35,7 @@ def step_impl(context): '"{default_value}"' ) def step_impl(context, flag_type, key, default_value): - context.client = get_client(name="Default Provider", version="1.0") + context.client = get_client() if flag_type == "boolean": context.boolean_flag_details = context.client.get_boolean_details( key, default_value diff --git a/tests/test_api.py b/tests/test_api.py index cb3cfecf..3756f85c 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -5,10 +5,10 @@ from openfeature.api import ( add_hooks, clear_hooks, + clear_providers, get_client, get_evaluation_context, get_hooks, - get_provider, get_provider_metadata, set_evaluation_context, set_provider, @@ -26,11 +26,9 @@ def test_should_not_raise_exception_with_noop_client(): # Given # No provider has been set # When - client = get_client(name="Default Provider", version="1.0") + client = get_client() # Then - assert client.name == "Default Provider" - assert client.version == "1.0" assert isinstance(client.provider, NoOpProvider) @@ -39,11 +37,9 @@ def test_should_return_open_feature_client_when_configured_correctly(): set_provider(NoOpProvider()) # When - client = get_client(name="No-op Provider", version="1.0") + client = get_client() # Then - assert client.name == "No-op Provider" - assert client.version == "1.0" assert isinstance(client.provider, NoOpProvider) @@ -84,18 +80,6 @@ def test_should_invoke_provider_shutdown_function_once_provider_is_no_longer_in_ assert provider_1.shutdown.called -def test_should_return_a_provider_if_setup_correctly(): - # Given - set_provider(NoOpProvider()) - - # When - provider = get_provider() - - # Then - assert provider - assert isinstance(provider, NoOpProvider) - - def test_should_retrieve_metadata_for_configured_provider(): # Given set_provider(NoOpProvider()) @@ -156,3 +140,91 @@ def test_should_call_provider_shutdown_on_api_shutdown(): # Then assert provider.shutdown.called + + +def test_should_provide_a_function_to_bind_provider_through_domain(): + # Given + provider = MagicMock(spec=FeatureProvider) + test_client = get_client("test") + default_client = get_client() + + # When + set_provider(provider, domain="test") + + # Then + assert default_client.provider != provider + assert default_client.domain is None + + assert test_client.provider == provider + assert test_client.domain == "test" + + +def test_should_not_initialize_provider_already_bound_to_another_domain(): + # Given + provider = MagicMock(spec=FeatureProvider) + set_provider(provider, "foo") + + # When + set_provider(provider, "bar") + + # Then + provider.initialize.assert_called_once() + + +def test_should_shutdown_unbound_provider(): + # Given + provider = MagicMock(spec=FeatureProvider) + set_provider(provider, "foo") + + # When + other_provider = MagicMock(spec=FeatureProvider) + set_provider(other_provider, "foo") + + provider.shutdown.assert_called_once() + + +def test_should_not_shutdown_provider_bound_to_another_domain(): + # Given + provider = MagicMock(spec=FeatureProvider) + set_provider(provider, "foo") + set_provider(provider, "bar") + + # When + other_provider = MagicMock(spec=FeatureProvider) + set_provider(other_provider, "foo") + + provider.shutdown.assert_not_called() + + +def test_shutdown_should_shutdown_every_registered_provider_once(): + # Given + provider_1 = MagicMock(spec=FeatureProvider) + provider_2 = MagicMock(spec=FeatureProvider) + set_provider(provider_1) + set_provider(provider_1, "foo") + set_provider(provider_2, "bar") + set_provider(provider_2, "baz") + + # When + shutdown() + + # Then + provider_1.shutdown.assert_called_once() + provider_2.shutdown.assert_called_once() + + +def test_clear_providers_shutdowns_every_provider_and_resets_default_provider(): + # Given + provider_1 = MagicMock(spec=FeatureProvider) + provider_2 = MagicMock(spec=FeatureProvider) + set_provider(provider_1) + set_provider(provider_2, "foo") + set_provider(provider_2, "bar") + + # When + clear_providers() + + # Then + provider_1.shutdown.assert_called_once() + provider_2.shutdown.assert_called_once() + assert isinstance(get_client().provider, NoOpProvider) diff --git a/tests/test_client.py b/tests/test_client.py index 1faed781..71873405 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -2,7 +2,7 @@ import pytest -from openfeature.api import add_hooks, clear_hooks +from openfeature.api import add_hooks, clear_hooks, set_provider from openfeature.client import OpenFeatureClient from openfeature.exception import ErrorCode, OpenFeatureError from openfeature.flag_evaluation import Reason @@ -109,7 +109,9 @@ def test_should_pass_flag_metadata_from_resolution_to_evaluation_details(): ) } ) - client = OpenFeatureClient("my-client", None, provider) + set_provider(provider, "my-client") + + client = OpenFeatureClient("my-client", None) # When details = client.get_boolean_details(flag_key="Key", default_value=False) @@ -158,14 +160,14 @@ def test_should_handle_an_open_feature_exception_thrown_by_a_provider( assert flag_details.error_message == "error_message" -def test_should_return_client_metadata_with_name(): +def test_should_return_client_metadata_with_domain(): # Given client = OpenFeatureClient("my-client", None, NoOpProvider()) # When metadata = client.get_metadata() # Then assert metadata is not None - assert metadata.name == "my-client" + assert metadata.domain == "my-client" def test_should_call_api_level_hooks(no_op_provider_client):