From 58d5551aefb78c244989e250905cc1ff281adf52 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Mon, 12 Jan 2026 09:54:05 -0500 Subject: [PATCH 1/6] fix: Validate domain is present when calling `set_provider` on registry It is incorrect to call `ProviderRegister.set_provider` without a provider AND a domain. A validation check exists for the provider, but none for the domain. In this commit, we introduce that domain validation and introduce tests to capture this expected behavior. Signed-off-by: Matthew Keeler --- openfeature/provider/_registry.py | 2 + tests/provider/test_registry.py | 118 ++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 tests/provider/test_registry.py diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index 7529d9fa..d3656c60 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -24,6 +24,8 @@ def __init__(self) -> None: def set_provider(self, domain: str, provider: FeatureProvider) -> None: if provider is None: raise GeneralError(error_message="No provider") + if domain is None: + raise GeneralError(error_message="No domain") providers = self._providers if domain in providers: old_provider = providers[domain] diff --git a/tests/provider/test_registry.py b/tests/provider/test_registry.py new file mode 100644 index 00000000..609739d2 --- /dev/null +++ b/tests/provider/test_registry.py @@ -0,0 +1,118 @@ +from unittest.mock import Mock + +from openfeature.exception import GeneralError +from openfeature.provider._registry import ProviderRegistry +from openfeature.provider.no_op_provider import NoOpProvider + + +def test_registry_serves_noop_as_default(): + registry = ProviderRegistry() + + assert isinstance(registry.get_default_provider(), NoOpProvider) + assert isinstance(registry.get_provider("unknown domain"), NoOpProvider) + + +def test_setting_provider_requires_domain(): + registry = ProviderRegistry() + + try: + registry.set_provider(None, NoOpProvider()) # type: ignore[reportArgumentType] + raise AssertionError( + "Expected an exception when setting provider with None domain" + ) + except GeneralError as e: + assert e.error_message == "No domain" + except Exception as e: + raise AssertionError("Expected GeneralError, got {type(e)}") from e + + +def test_setting_provider_requires_provider(): + registry = ProviderRegistry() + + try: + registry.set_provider("domain", None) # type: ignore[reportArgumentType] + raise AssertionError("Expected an exception when setting provider to None") + except GeneralError as e: + assert e.error_message == "No provider" + except Exception as e: + raise AssertionError("Expected GeneralError, got {type(e)}") from e + + +def test_can_register_provider_to_multiple_domains(): + registry = ProviderRegistry() + provider = NoOpProvider() + + registry.set_provider("domain1", provider) + registry.set_provider("domain2", provider) + + assert registry.get_provider("domain1") is provider + assert registry.get_provider("domain2") is provider + + +def test_registering_provider_replaces_previous_provider(): + """Test that registering a provider replaces the previous provider and calls shutdown on the old one.""" + + registry = ProviderRegistry() + provider1 = Mock() + provider2 = Mock() + + registry.set_provider("domain", provider1) + assert registry.get_provider("domain") is provider1 + + registry.set_provider("domain", provider2) + assert registry.get_provider("domain") is provider2 + + provider1.shutdown.assert_called_once() + provider2.shutdown.assert_not_called() + + +def test_registering_provider_for_first_time_initializes_it(): + """Test that registering a provider for the first time calls its initialize method.""" + + registry = ProviderRegistry() + provider = Mock() + + registry.set_provider("domain1", provider) + registry.set_provider("domain2", provider) + + provider.initialize.assert_called_once() + + +def test_setting_default_provider_requires_provider(): + registry = ProviderRegistry() + + try: + registry.set_default_provider(None) # type: ignore[reportArgumentType] + raise AssertionError( + "Expected an exception when setting default provider to None" + ) + except GeneralError as e: + assert e.error_message == "No provider" + except Exception as e: + raise AssertionError("Expected GeneralError, got {type(e)}") from e + + +def test_replacing_default_provider_shuts_down_old_one(): + """Test that replacing the default provider shuts down the old default provider.""" + + registry = ProviderRegistry() + default_provider1 = Mock() + default_provider2 = Mock() + + registry.set_default_provider(default_provider1) + assert registry.get_default_provider() is default_provider1 + + registry.set_default_provider(default_provider2) + assert registry.get_default_provider() is default_provider2 + + default_provider1.shutdown.assert_called_once() + default_provider2.shutdown.assert_not_called() + + +def test_setting_default_provider_initializes_it(): + registry = ProviderRegistry() + provider = Mock() + + registry.set_default_provider(provider) + + provider.initialize.assert_called_once() From 69b749f01434a92e497f52a814e465c772d66417 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Mon, 12 Jan 2026 10:33:10 -0500 Subject: [PATCH 2/6] fix: Prevent providers from being initialized multiple times If a provider is set for two different domains, it will only be initialized once. However, if a customer were to use a provider as both default and domain specific, it would incorrectly be initialized twice. Signed-off-by: Matthew Keeler --- openfeature/provider/_registry.py | 6 ++++-- tests/provider/test_registry.py | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index d3656c60..63fb4cf0 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -32,7 +32,7 @@ def set_provider(self, domain: str, provider: FeatureProvider) -> None: del providers[domain] if old_provider not in providers.values(): self._shutdown_provider(old_provider) - if provider not in providers.values(): + if provider not in providers.values() and provider != self._default_provider: self._initialize_provider(provider) providers[domain] = provider @@ -47,7 +47,9 @@ def set_default_provider(self, provider: FeatureProvider) -> None: if self._default_provider: self._shutdown_provider(self._default_provider) self._default_provider = provider - self._initialize_provider(provider) + + if self._default_provider not in self._providers.values(): + self._initialize_provider(provider) def get_default_provider(self) -> FeatureProvider: return self._default_provider diff --git a/tests/provider/test_registry.py b/tests/provider/test_registry.py index 609739d2..29ce829d 100644 --- a/tests/provider/test_registry.py +++ b/tests/provider/test_registry.py @@ -116,3 +116,27 @@ def test_setting_default_provider_initializes_it(): registry.set_default_provider(provider) provider.initialize.assert_called_once() + + +def test_registering_provider_as_default_then_domain_only_initializes_once(): + """Test that registering the same provider as default and for a domain only initializes it once.""" + + registry = ProviderRegistry() + provider = Mock() + + registry.set_default_provider(provider) + registry.set_provider("domain", provider) + + provider.initialize.assert_called_once() + + +def test_registering_provider_as_domain_then_default_only_initializes_once(): + """Test that registering the same provider as default and for a domain only initializes it once.""" + + registry = ProviderRegistry() + provider = Mock() + + registry.set_provider("domain", provider) + registry.set_default_provider(provider) + + provider.initialize.assert_called_once() From 1663a58e2b80bb4a9cf911eb3189701fe5470ff1 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Mon, 12 Jan 2026 10:41:25 -0500 Subject: [PATCH 3/6] fix: Prevent providers from being shutdown multiple times If a provider is set for two different domains, and then one of them is replaced, it will only be shutdown once. However, if a customer were to use a provider as both default and domain specific, replacing one or the other would incorrectly shutdown both usages. Signed-off-by: Matthew Keeler --- openfeature/provider/_registry.py | 10 ++++++++-- tests/provider/test_registry.py | 32 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index 63fb4cf0..de346007 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -30,7 +30,10 @@ def set_provider(self, domain: str, provider: FeatureProvider) -> None: if domain in providers: old_provider = providers[domain] del providers[domain] - if old_provider not in providers.values(): + if ( + old_provider not in providers.values() + and old_provider != self._default_provider + ): self._shutdown_provider(old_provider) if provider not in providers.values() and provider != self._default_provider: self._initialize_provider(provider) @@ -44,7 +47,10 @@ def get_provider(self, domain: str | None) -> FeatureProvider: def set_default_provider(self, provider: FeatureProvider) -> None: if provider is None: raise GeneralError(error_message="No provider") - if self._default_provider: + if ( + self._default_provider + and self._default_provider not in self._providers.values() + ): self._shutdown_provider(self._default_provider) self._default_provider = provider diff --git a/tests/provider/test_registry.py b/tests/provider/test_registry.py index 29ce829d..3ca39973 100644 --- a/tests/provider/test_registry.py +++ b/tests/provider/test_registry.py @@ -140,3 +140,35 @@ def test_registering_provider_as_domain_then_default_only_initializes_once(): registry.set_default_provider(provider) provider.initialize.assert_called_once() + + +def test_replacing_provider_used_as_default_does_not_shutdown(): + """Test that replacing a provider that is also the default does not shut it down twice.""" + + registry = ProviderRegistry() + provider1 = Mock() + provider2 = Mock() + + registry.set_default_provider(provider1) + registry.set_provider("domain", provider1) + + registry.set_provider("domain", provider2) + + provider1.shutdown.assert_not_called() + provider2.shutdown.assert_not_called() + + +def test_replacing_default_provider_used_as_domain_does_not_shutdown(): + """Test that replacing a default provider that is also used for a domain does not shut it down twice.""" + + registry = ProviderRegistry() + provider1 = Mock() + provider2 = Mock() + + registry.set_provider("domain", provider1) + registry.set_default_provider(provider1) + + registry.set_default_provider(provider2) + + provider1.shutdown.assert_not_called() + provider2.shutdown.assert_not_called() From f7998e6fbc2a4c926991991230d33542f3ff57ad Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Mon, 12 Jan 2026 11:04:27 -0500 Subject: [PATCH 4/6] refactor: Delete provider status instead of marking as NOT_READY Once a provider has been removed, there is no value in retaining a reference to it in the `_provider_status` dictionary. Instead of explicitly setting the status as `NOT_READY`, we can rely on this being the default status served from the `get_provider_status` method, allowing us to remove the dictionary entry entirely. Signed-off-by: Matthew Keeler --- openfeature/provider/_registry.py | 2 +- tests/provider/test_registry.py | 67 +++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index de346007..3f82648a 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -102,7 +102,7 @@ def _shutdown_provider(self, provider: FeatureProvider) -> None: try: if hasattr(provider, "shutdown"): provider.shutdown() - self._provider_status[provider] = ProviderStatus.NOT_READY + del self._provider_status[provider] except Exception as err: self.dispatch_event( provider, diff --git a/tests/provider/test_registry.py b/tests/provider/test_registry.py index 3ca39973..21bae389 100644 --- a/tests/provider/test_registry.py +++ b/tests/provider/test_registry.py @@ -1,6 +1,7 @@ from unittest.mock import Mock from openfeature.exception import GeneralError +from openfeature.provider import ProviderStatus from openfeature.provider._registry import ProviderRegistry from openfeature.provider.no_op_provider import NoOpProvider @@ -172,3 +173,69 @@ def test_replacing_default_provider_used_as_domain_does_not_shutdown(): provider1.shutdown.assert_not_called() provider2.shutdown.assert_not_called() + + +def test_shutting_down_registry_shuts_down_providers_once(): + """Test that shutting down the registry shuts down each provider only once.""" + + registry = ProviderRegistry() + provider1 = Mock() + provider2 = Mock() + + registry.set_default_provider(provider1) + registry.set_provider("domain1", provider1) + + registry.set_provider("domain2a", provider2) + registry.set_provider("domain2b", provider2) + + registry.shutdown() + + provider1.shutdown.assert_called_once() + provider2.shutdown.assert_called_once() + + +def test_initializing_provider_sets_status_ready(): + """Test that initializing a provider sets its status to READY.""" + + registry = ProviderRegistry() + provider = Mock() + + assert registry.get_provider_status(provider) == ProviderStatus.NOT_READY + + registry.set_provider("domain", provider) + + provider.initialize.assert_called_once() + assert registry.get_provider_status(provider) == ProviderStatus.READY + + +def test_shutting_down_provider_sets_status_not_ready(): + """Test that shutting down a provider sets its status to NOT_READY.""" + + registry = ProviderRegistry() + provider = Mock() + + registry.set_provider("domain", provider) + assert registry.get_provider_status(provider) == ProviderStatus.READY + + registry.shutdown() + assert registry.get_provider_status(provider) == ProviderStatus.NOT_READY + + +def test_clearing_registry_resets_providers_and_default(): + """Test that clearing the registry resets all providers and the default provider.""" + + registry = ProviderRegistry() + provider = Mock() + + registry.set_provider("domain", provider) + registry.set_default_provider(provider) + + registry.clear_providers() + + default_provider = registry.get_default_provider() + assert isinstance(default_provider, NoOpProvider) + assert registry.get_provider("domain") is default_provider + assert registry.get_provider_status(default_provider) == ProviderStatus.READY + + provider.initialize.assert_called_once() + provider.shutdown.assert_called_once() From 69076e2c8c2de51ab955b4145ee481d3eb07a911 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 13 Jan 2026 08:33:51 -0500 Subject: [PATCH 5/6] Swap order for faster comparison Signed-off-by: Matthew Keeler --- openfeature/provider/_registry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index 3f82648a..d0bf2358 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -31,11 +31,11 @@ def set_provider(self, domain: str, provider: FeatureProvider) -> None: old_provider = providers[domain] del providers[domain] if ( + old_provider != self._default_provider and old_provider not in providers.values() - and old_provider != self._default_provider ): self._shutdown_provider(old_provider) - if provider not in providers.values() and provider != self._default_provider: + if provider != self._default_provider and provider not in providers.values(): self._initialize_provider(provider) providers[domain] = provider From 4550e5353a5efa55ab5f34a495bef5005695a394 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Tue, 13 Jan 2026 08:36:58 -0500 Subject: [PATCH 6/6] formatting --- openfeature/provider/_registry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openfeature/provider/_registry.py b/openfeature/provider/_registry.py index d0bf2358..bf8fa9a8 100644 --- a/openfeature/provider/_registry.py +++ b/openfeature/provider/_registry.py @@ -31,8 +31,8 @@ def set_provider(self, domain: str, provider: FeatureProvider) -> None: old_provider = providers[domain] del providers[domain] if ( - old_provider != self._default_provider and - old_provider not in providers.values() + old_provider != self._default_provider + and old_provider not in providers.values() ): self._shutdown_provider(old_provider) if provider != self._default_provider and provider not in providers.values():