Skip to content

Commit 58d5551

Browse files
committed
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 <mkeeler@launchdarkly.com>
1 parent 7c27c7a commit 58d5551

2 files changed

Lines changed: 120 additions & 0 deletions

File tree

openfeature/provider/_registry.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ def __init__(self) -> None:
2424
def set_provider(self, domain: str, provider: FeatureProvider) -> None:
2525
if provider is None:
2626
raise GeneralError(error_message="No provider")
27+
if domain is None:
28+
raise GeneralError(error_message="No domain")
2729
providers = self._providers
2830
if domain in providers:
2931
old_provider = providers[domain]

tests/provider/test_registry.py

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
from unittest.mock import Mock
2+
3+
from openfeature.exception import GeneralError
4+
from openfeature.provider._registry import ProviderRegistry
5+
from openfeature.provider.no_op_provider import NoOpProvider
6+
7+
8+
def test_registry_serves_noop_as_default():
9+
registry = ProviderRegistry()
10+
11+
assert isinstance(registry.get_default_provider(), NoOpProvider)
12+
assert isinstance(registry.get_provider("unknown domain"), NoOpProvider)
13+
14+
15+
def test_setting_provider_requires_domain():
16+
registry = ProviderRegistry()
17+
18+
try:
19+
registry.set_provider(None, NoOpProvider()) # type: ignore[reportArgumentType]
20+
raise AssertionError(
21+
"Expected an exception when setting provider with None domain"
22+
)
23+
except GeneralError as e:
24+
assert e.error_message == "No domain"
25+
except Exception as e:
26+
raise AssertionError("Expected GeneralError, got {type(e)}") from e
27+
28+
29+
def test_setting_provider_requires_provider():
30+
registry = ProviderRegistry()
31+
32+
try:
33+
registry.set_provider("domain", None) # type: ignore[reportArgumentType]
34+
raise AssertionError("Expected an exception when setting provider to None")
35+
except GeneralError as e:
36+
assert e.error_message == "No provider"
37+
except Exception as e:
38+
raise AssertionError("Expected GeneralError, got {type(e)}") from e
39+
40+
41+
def test_can_register_provider_to_multiple_domains():
42+
registry = ProviderRegistry()
43+
provider = NoOpProvider()
44+
45+
registry.set_provider("domain1", provider)
46+
registry.set_provider("domain2", provider)
47+
48+
assert registry.get_provider("domain1") is provider
49+
assert registry.get_provider("domain2") is provider
50+
51+
52+
def test_registering_provider_replaces_previous_provider():
53+
"""Test that registering a provider replaces the previous provider and calls shutdown on the old one."""
54+
55+
registry = ProviderRegistry()
56+
provider1 = Mock()
57+
provider2 = Mock()
58+
59+
registry.set_provider("domain", provider1)
60+
assert registry.get_provider("domain") is provider1
61+
62+
registry.set_provider("domain", provider2)
63+
assert registry.get_provider("domain") is provider2
64+
65+
provider1.shutdown.assert_called_once()
66+
provider2.shutdown.assert_not_called()
67+
68+
69+
def test_registering_provider_for_first_time_initializes_it():
70+
"""Test that registering a provider for the first time calls its initialize method."""
71+
72+
registry = ProviderRegistry()
73+
provider = Mock()
74+
75+
registry.set_provider("domain1", provider)
76+
registry.set_provider("domain2", provider)
77+
78+
provider.initialize.assert_called_once()
79+
80+
81+
def test_setting_default_provider_requires_provider():
82+
registry = ProviderRegistry()
83+
84+
try:
85+
registry.set_default_provider(None) # type: ignore[reportArgumentType]
86+
raise AssertionError(
87+
"Expected an exception when setting default provider to None"
88+
)
89+
except GeneralError as e:
90+
assert e.error_message == "No provider"
91+
except Exception as e:
92+
raise AssertionError("Expected GeneralError, got {type(e)}") from e
93+
94+
95+
def test_replacing_default_provider_shuts_down_old_one():
96+
"""Test that replacing the default provider shuts down the old default provider."""
97+
98+
registry = ProviderRegistry()
99+
default_provider1 = Mock()
100+
default_provider2 = Mock()
101+
102+
registry.set_default_provider(default_provider1)
103+
assert registry.get_default_provider() is default_provider1
104+
105+
registry.set_default_provider(default_provider2)
106+
assert registry.get_default_provider() is default_provider2
107+
108+
default_provider1.shutdown.assert_called_once()
109+
default_provider2.shutdown.assert_not_called()
110+
111+
112+
def test_setting_default_provider_initializes_it():
113+
registry = ProviderRegistry()
114+
provider = Mock()
115+
116+
registry.set_default_provider(provider)
117+
118+
provider.initialize.assert_called_once()

0 commit comments

Comments
 (0)