Skip to content

Commit 6f95616

Browse files
committed
fix provider mulitple regiration issue
Signed-off-by: Kavindu Dodanduwa <[email protected]>
1 parent 1fd11c4 commit 6f95616

File tree

5 files changed

+52
-9
lines changed

5 files changed

+52
-9
lines changed

src/main/java/dev/openfeature/sdk/EventProvider.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ public abstract class EventProvider implements FeatureProvider {
2828
* "Attach" this EventProvider to an SDK, which allows events to propagate from this provider.
2929
* No-op if the same onEmit is already attached.
3030
*
31-
* @param onEmit the function to run when a provider emits events.
31+
* @param onEmit the function to run when a provider emits events.
32+
* @throws IllegalStateException if attempted to bind a new emitter for already bound provider
33+
*
3234
*/
3335
void attach(TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit) {
3436
if (this.onEmit != null && this.onEmit != onEmit) {

src/main/java/dev/openfeature/sdk/ProviderRepository.java

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,14 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
108108

109109
// provider is set immediately, on this thread
110110
FeatureProvider oldProvider = clientName != null
111-
? this.providers.put(clientName, newProvider)
112-
: this.defaultProvider.getAndSet(newProvider);
113-
afterSet.accept(newProvider);
111+
? this.providers.put(clientName, newProvider)
112+
: this.defaultProvider.getAndSet(newProvider);
113+
114+
if (!isProviderRegistered(newProvider)) {
115+
// only run afterSet if new provider is not already attached
116+
afterSet.accept(newProvider);
117+
}
118+
114119
if (waitForInit) {
115120
initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider);
116121
} else {
@@ -138,16 +143,21 @@ private void initializeProvider(FeatureProvider newProvider,
138143
}
139144
}
140145

141-
private void shutDownOld(FeatureProvider oldProvider,Consumer<FeatureProvider> afterShutdown) {
142-
if (oldProvider != null && !isProviderRegistered(oldProvider)) {
146+
private void shutDownOld(FeatureProvider oldProvider, Consumer<FeatureProvider> afterShutdown) {
147+
if (!isProviderRegistered(oldProvider)) {
143148
shutdownProvider(oldProvider);
144149
afterShutdown.accept(oldProvider);
145150
}
146151
}
147152

148-
private boolean isProviderRegistered(FeatureProvider oldProvider) {
149-
return oldProvider != null && (this.providers.containsValue(oldProvider)
150-
|| this.defaultProvider.get().equals(oldProvider));
153+
/**
154+
* Helper to check if provider is already known (registered)
155+
* @param provider provider to check for registration
156+
* @return boolean true if already registered, false otherwise
157+
*/
158+
private boolean isProviderRegistered(FeatureProvider provider) {
159+
return provider != null &&
160+
(this.providers.containsValue(provider) || this.defaultProvider.get().equals(provider));
151161
}
152162

153163
private void shutdownProvider(FeatureProvider provider) {

src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
package dev.openfeature.sdk;
22

3+
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
34
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
45
import org.junit.jupiter.api.BeforeEach;
56
import org.junit.jupiter.api.Test;
67

8+
import java.util.Collections;
9+
710
import static org.assertj.core.api.Assertions.assertThat;
811
import static org.assertj.core.api.Assertions.assertThatCode;
12+
import static org.junit.jupiter.api.Assertions.assertEquals;
913

1014
class OpenFeatureAPITest {
1115

@@ -40,6 +44,20 @@ void namedProviderOverwrittenTest() {
4044
.isEqualTo(DoSomethingProvider.name);
4145
}
4246

47+
@Test
48+
void providerToMultipleNames() {
49+
FeatureProvider provider = new InMemoryProvider(Collections.EMPTY_MAP);
50+
51+
// register same provider for multiple names & as default provider
52+
OpenFeatureAPI.getInstance().setProviderAndWait(provider);
53+
OpenFeatureAPI.getInstance().setProviderAndWait("clientA", provider);
54+
OpenFeatureAPI.getInstance().setProviderAndWait("clientB", provider);
55+
56+
assertEquals(provider, OpenFeatureAPI.getInstance().getProvider());
57+
assertEquals(provider, OpenFeatureAPI.getInstance().getProvider("clientA"));
58+
assertEquals(provider, OpenFeatureAPI.getInstance().getProvider("clientB"));
59+
}
60+
4361
@Test
4462
void settingDefaultProviderToNullErrors() {
4563
assertThatCode(() -> api.setProvider(null)).isInstanceOf(IllegalArgumentException.class);

src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import static org.awaitility.Awaitility.await;
1010
import static org.mockito.ArgumentMatchers.any;
1111
import static org.mockito.ArgumentMatchers.eq;
12+
import static org.mockito.Mockito.atMostOnce;
1213
import static org.mockito.Mockito.doThrow;
1314
import static org.mockito.Mockito.mock;
1415
import static org.mockito.Mockito.never;
@@ -140,6 +141,17 @@ void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready(
140141

141142
verify(provider, never()).initialize(any());
142143
}
144+
145+
@Test
146+
@DisplayName("Should allow same provider to be registered with multiple names")
147+
void allowSameProviderOnMultipleNames() throws Exception {
148+
FeatureProvider provider = createMockedProvider();
149+
150+
setFeatureProvider(CLIENT_NAME, provider);
151+
setFeatureProvider(ANOTHER_CLIENT_NAME, provider);
152+
153+
verify(provider, atMostOnce()).initialize(any());
154+
}
143155
}
144156
}
145157

src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import static org.awaitility.Awaitility.await;
1010

11+
// todo check the need of this utility class as we now have setProviderAndWait capability
1112
@UtilityClass
1213
public class FeatureProviderTestUtils {
1314

0 commit comments

Comments
 (0)