Skip to content

fix: tolerate duplicate provider registrations #725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@
default="checkstyle-xpath-suppressions.xml" />
<property name="optional" value="true"/>
</module>


<module name="UnusedImports"/>
<module name="OuterTypeFilename"/>
<module name="IllegalTokenText">
<property name="tokens" value="STRING_LITERAL, CHAR_LITERAL"/>
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/dev/openfeature/sdk/EventProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ public abstract class EventProvider implements FeatureProvider {
* "Attach" this EventProvider to an SDK, which allows events to propagate from this provider.
* No-op if the same onEmit is already attached.
*
* @param onEmit the function to run when a provider emits events.
* @param onEmit the function to run when a provider emits events.
* @throws IllegalStateException if attempted to bind a new emitter for already bound provider
*
*/
void attach(TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit) {
if (this.onEmit != null && this.onEmit != onEmit) {
Expand Down
26 changes: 18 additions & 8 deletions src/main/java/dev/openfeature/sdk/ProviderRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,16 @@ private void prepareAndInitializeProvider(@Nullable String clientName,
BiConsumer<FeatureProvider, String> afterError,
boolean waitForInit) {

if (!isProviderRegistered(newProvider)) {
// only run afterSet if new provider is not already attached
afterSet.accept(newProvider);
}

// provider is set immediately, on this thread
FeatureProvider oldProvider = clientName != null
? this.providers.put(clientName, newProvider)
: this.defaultProvider.getAndSet(newProvider);
afterSet.accept(newProvider);
? this.providers.put(clientName, newProvider)
: this.defaultProvider.getAndSet(newProvider);

if (waitForInit) {
initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider);
} else {
Expand Down Expand Up @@ -138,16 +143,21 @@ private void initializeProvider(FeatureProvider newProvider,
}
}

private void shutDownOld(FeatureProvider oldProvider,Consumer<FeatureProvider> afterShutdown) {
if (oldProvider != null && !isProviderRegistered(oldProvider)) {
private void shutDownOld(FeatureProvider oldProvider, Consumer<FeatureProvider> afterShutdown) {
if (!isProviderRegistered(oldProvider)) {
shutdownProvider(oldProvider);
afterShutdown.accept(oldProvider);
}
}

private boolean isProviderRegistered(FeatureProvider oldProvider) {
return oldProvider != null && (this.providers.containsValue(oldProvider)
|| this.defaultProvider.get().equals(oldProvider));
/**
* Helper to check if provider is already known (registered).
* @param provider provider to check for registration
* @return boolean true if already registered, false otherwise
*/
private boolean isProviderRegistered(FeatureProvider provider) {
return provider != null
&& (this.providers.containsValue(provider) || this.defaultProvider.get().equals(provider));
}

private void shutdownProvider(FeatureProvider provider) {
Expand Down
23 changes: 23 additions & 0 deletions src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package dev.openfeature.sdk;

import dev.openfeature.sdk.providers.memory.InMemoryProvider;
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.Collections;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.junit.jupiter.api.Assertions.assertEquals;

class OpenFeatureAPITest {

Expand Down Expand Up @@ -40,6 +44,25 @@ void namedProviderOverwrittenTest() {
.isEqualTo(DoSomethingProvider.name);
}

@Test
void providerToMultipleNames() {
FeatureProvider inMemAsEventingProvider = new InMemoryProvider(Collections.EMPTY_MAP);
FeatureProvider noOpAsNonEventingProvider = new NoOpProvider();

// register same provider for multiple names & as default provider
OpenFeatureAPI.getInstance().setProviderAndWait(inMemAsEventingProvider);
OpenFeatureAPI.getInstance().setProviderAndWait("clientA", inMemAsEventingProvider);
OpenFeatureAPI.getInstance().setProviderAndWait("clientB", inMemAsEventingProvider);
OpenFeatureAPI.getInstance().setProviderAndWait("clientC", noOpAsNonEventingProvider);
OpenFeatureAPI.getInstance().setProviderAndWait("clientD", noOpAsNonEventingProvider);

assertEquals(inMemAsEventingProvider, OpenFeatureAPI.getInstance().getProvider());
assertEquals(inMemAsEventingProvider, OpenFeatureAPI.getInstance().getProvider("clientA"));
assertEquals(inMemAsEventingProvider, OpenFeatureAPI.getInstance().getProvider("clientB"));
assertEquals(noOpAsNonEventingProvider, OpenFeatureAPI.getInstance().getProvider("clientC"));
assertEquals(noOpAsNonEventingProvider, OpenFeatureAPI.getInstance().getProvider("clientD"));
}

@Test
void settingDefaultProviderToNullErrors() {
assertThatCode(() -> api.setProvider(null)).isInstanceOf(IllegalArgumentException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import static org.awaitility.Awaitility.await;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.atMostOnce;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import static org.awaitility.Awaitility.await;

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

Expand Down