Skip to content

Commit 3319e55

Browse files
Kavindu-DodanKavindu-Dodan
authored
fix: tolerate duplicate provider registrations (open-feature#725)
* fix provider mulitple regiration issue Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com> * fix lint Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com> * fix tests Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com> * improve test and add check for unused imports Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com> --------- Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
1 parent 07ea4c0 commit 3319e55

File tree

6 files changed

+48
-10
lines changed

6 files changed

+48
-10
lines changed

‎checkstyle.xml

+2-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@
6363
default="checkstyle-xpath-suppressions.xml" />
6464
<property name="optional" value="true"/>
6565
</module>
66-
66+
67+
<module name="UnusedImports"/>
6768
<module name="OuterTypeFilename"/>
6869
<module name="IllegalTokenText">
6970
<property name="tokens" value="STRING_LITERAL, CHAR_LITERAL"/>

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

+3-1
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

+18-8
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,16 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
106106
BiConsumer<FeatureProvider, String> afterError,
107107
boolean waitForInit) {
108108

109+
if (!isProviderRegistered(newProvider)) {
110+
// only run afterSet if new provider is not already attached
111+
afterSet.accept(newProvider);
112+
}
113+
109114
// provider is set immediately, on this thread
110115
FeatureProvider oldProvider = clientName != null
111-
? this.providers.put(clientName, newProvider)
112-
: this.defaultProvider.getAndSet(newProvider);
113-
afterSet.accept(newProvider);
116+
? this.providers.put(clientName, newProvider)
117+
: this.defaultProvider.getAndSet(newProvider);
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

+23
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,25 @@ void namedProviderOverwrittenTest() {
4044
.isEqualTo(DoSomethingProvider.name);
4145
}
4246

47+
@Test
48+
void providerToMultipleNames() {
49+
FeatureProvider inMemAsEventingProvider = new InMemoryProvider(Collections.EMPTY_MAP);
50+
FeatureProvider noOpAsNonEventingProvider = new NoOpProvider();
51+
52+
// register same provider for multiple names & as default provider
53+
OpenFeatureAPI.getInstance().setProviderAndWait(inMemAsEventingProvider);
54+
OpenFeatureAPI.getInstance().setProviderAndWait("clientA", inMemAsEventingProvider);
55+
OpenFeatureAPI.getInstance().setProviderAndWait("clientB", inMemAsEventingProvider);
56+
OpenFeatureAPI.getInstance().setProviderAndWait("clientC", noOpAsNonEventingProvider);
57+
OpenFeatureAPI.getInstance().setProviderAndWait("clientD", noOpAsNonEventingProvider);
58+
59+
assertEquals(inMemAsEventingProvider, OpenFeatureAPI.getInstance().getProvider());
60+
assertEquals(inMemAsEventingProvider, OpenFeatureAPI.getInstance().getProvider("clientA"));
61+
assertEquals(inMemAsEventingProvider, OpenFeatureAPI.getInstance().getProvider("clientB"));
62+
assertEquals(noOpAsNonEventingProvider, OpenFeatureAPI.getInstance().getProvider("clientC"));
63+
assertEquals(noOpAsNonEventingProvider, OpenFeatureAPI.getInstance().getProvider("clientD"));
64+
}
65+
4366
@Test
4467
void settingDefaultProviderToNullErrors() {
4568
assertThatCode(() -> api.setProvider(null)).isInstanceOf(IllegalArgumentException.class);

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

+1
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;

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

+1
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)