Skip to content
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

feat: add method to set provider and block during init #563

Merged
merged 10 commits into from
Aug 28, 2023
55 changes: 45 additions & 10 deletions src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,12 @@
public void setProvider(FeatureProvider provider) {
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
providerRepository.setProvider(
provider,
(p) -> attachEventProvider(p),
(p) -> emitReady(p),
(p) -> detachEventProvider(p),
(p, message) -> emitError(p, message));
provider,
(p) -> attachEventProvider(p),
(p) -> emitReady(p),
(p) -> detachEventProvider(p),
(p, message) -> emitError(p, message),
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
false);
}
}

Expand All @@ -117,11 +118,45 @@
public void setProvider(String clientName, FeatureProvider provider) {
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
providerRepository.setProvider(clientName,
provider,
this::attachEventProvider,
this::emitReady,
this::detachEventProvider,
this::emitError);
provider,
this::attachEventProvider,
this::emitReady,
this::detachEventProvider,
this::emitError,
false);
}
}

/**
* Set the default provider and wait for initialization to finish.
*/
public void setProviderAndWait(FeatureProvider provider) {
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
providerRepository.setProvider(
provider,
(p) -> attachEventProvider(p),
(p) -> emitReady(p),
(p) -> detachEventProvider(p),
(p, message) -> emitError(p, message),

Check warning on line 140 in src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java#L140

Added line #L140 was not covered by tests
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
true);
}
}

/**
* Add a provider for a named client and wait for initialization to finish.
*
* @param clientName The name of the client.
* @param provider The provider to set.
*/
public void setProviderAndWait(String clientName, FeatureProvider provider) {
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
providerRepository.setProvider(clientName,
provider,
this::attachEventProvider,
this::emitReady,
this::detachEventProvider,
this::emitError,
true);
}
}

Expand Down
84 changes: 51 additions & 33 deletions src/main/java/dev/openfeature/sdk/ProviderRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ public FeatureProvider getProvider(String name) {

public List<String> getClientNamesForProvider(FeatureProvider provider) {
return providers.entrySet().stream()
.filter(entry -> entry.getValue().equals(provider))
.map(entry -> entry.getKey()).collect(Collectors.toList());
.filter(entry -> entry.getValue().equals(provider))
.map(entry -> entry.getKey()).collect(Collectors.toList());
}

public Set<String> getAllBoundClientNames() {
Expand All @@ -62,58 +62,76 @@ public void setProvider(FeatureProvider provider,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError) {
BiConsumer<FeatureProvider, String> afterError,
boolean waitForInit) {
if (provider == null) {
throw new IllegalArgumentException("Provider cannot be null");
}
initializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError);
prepareAndInitializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit);
}

/**
* Add a provider for a named client.
*
* @param clientName The name of the client.
* @param provider The provider to set.
* @param clientName The name of the client.
* @param provider The provider to set.
* @param waitForInit When true, wait for initialization to finish, then returns.
* Otherwise, initialization happens in the background.
*/
public void setProvider(String clientName,
FeatureProvider provider,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError) {
FeatureProvider provider,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError,
boolean waitForInit) {
if (provider == null) {
throw new IllegalArgumentException("Provider cannot be null");
}
if (clientName == null) {
throw new IllegalArgumentException("clientName cannot be null");
}
initializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError);
prepareAndInitializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit);
}

private void initializeProvider(@Nullable String clientName,
FeatureProvider newProvider,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError) {
private void prepareAndInitializeProvider(@Nullable String clientName,
FeatureProvider newProvider,
Consumer<FeatureProvider> afterSet,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError,
boolean waitForInit) {

// provider is set immediately, on this thread
FeatureProvider oldProvider = clientName != null
? this.providers.put(clientName, newProvider)
: this.defaultProvider.getAndSet(newProvider);
? this.providers.put(clientName, newProvider)
: this.defaultProvider.getAndSet(newProvider);
afterSet.accept(newProvider);
taskExecutor.submit(() -> {
// initialization happens in a different thread
try {
if (ProviderState.NOT_READY.equals(newProvider.getState())) {
newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
afterInit.accept(newProvider);
}
shutDownOld(oldProvider, afterShutdown);
} catch (Exception e) {
log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
afterError.accept(newProvider, e.getMessage());
if (waitForInit) {
initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider);
} else {
taskExecutor.submit(() -> {
// initialization happens in a different thread if we're not waiting it
initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider);
});
}
}

private void initializeProvider(FeatureProvider newProvider,
Consumer<FeatureProvider> afterInit,
Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError,
FeatureProvider oldProvider) {
try {
if (ProviderState.NOT_READY.equals(newProvider.getState())) {
newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
afterInit.accept(newProvider);
}
});
shutDownOld(oldProvider, afterShutdown);
} catch (Exception e) {
log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
afterError.accept(newProvider, e.getMessage());
}
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
}

private void shutDownOld(FeatureProvider oldProvider,Consumer<FeatureProvider> afterShutdown) {
Expand Down Expand Up @@ -157,7 +175,7 @@ public void shutdown() {
},
(FeatureProvider fp,
String message) -> {
});
}, false);
this.providers.clear();
taskExecutor.shutdown();
}
Expand Down
33 changes: 33 additions & 0 deletions src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
import java.util.List;
import java.util.Map;

import dev.openfeature.sdk.providers.memory.InMemoryProvider;
import dev.openfeature.sdk.testutils.TestEventsProvider;
import lombok.SneakyThrows;
import org.awaitility.Awaitility;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -69,6 +73,34 @@ void getApiInstance() {
assertThat(api.getProvider()).isEqualTo(mockProvider);
}

@SneakyThrows
@Specification(number="1.1.8", text="The API SHOULD provide functions to set a provider and wait for the initialize function to return or throw.")
@Test void providerAndWait() {
FeatureProvider provider = new TestEventsProvider(500);
OpenFeatureAPI.getInstance().setProviderAndWait(provider);
assertThat(api.getProvider().getState()).isEqualTo(ProviderState.READY);

provider = new TestEventsProvider(500);
String providerName = "providerAndWait";
OpenFeatureAPI.getInstance().setProviderAndWait(providerName, provider);
assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY);
}

@Specification(number="2.4.5", text="The provider SHOULD indicate an error if flag resolution is attempted before the provider is ready.")
@Test void shouldReturnNotReadyIfNotInitialized() {
FeatureProvider provider = new InMemoryProvider(new HashMap<>()) {
@Override
public void initialize(EvaluationContext evaluationContext) throws Exception {
Awaitility.await().wait(3000);
}
};
String providerName = "shouldReturnNotReadyIfNotInitialized";
OpenFeatureAPI.getInstance().setProvider(providerName, provider);
assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.NOT_READY);
Client client = OpenFeatureAPI.getInstance().getClient(providerName);
assertEquals(ErrorCode.PROVIDER_NOT_READY, client.getBooleanDetails("return_error_when_not_initialized", false).getErrorCode());
}

@Specification(number="1.1.5", text="The API MUST provide a function for retrieving the metadata field of the configured provider.")
@Test void provider_metadata() {
FeatureProviderTestUtils.setFeatureProvider(new DoSomethingProvider());
Expand Down Expand Up @@ -291,4 +323,5 @@ void getApiInstance() {

@Specification(number="1.4.11", text="The client SHOULD provide asynchronous or non-blocking mechanisms for flag evaluation.")
@Test void one_thread_per_request_model() {}

}
20 changes: 10 additions & 10 deletions src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class DefaultProvider {
@DisplayName("should reject null as default provider")
void shouldRejectNullAsDefaultProvider() {
assertThatCode(() -> providerRepository.setProvider(null, mockAfterSet(), mockAfterInit(),
mockAfterShutdown(), mockAfterError())).isInstanceOf(IllegalArgumentException.class);
mockAfterShutdown(), mockAfterError(), false)).isInstanceOf(IllegalArgumentException.class);
}

@Test
Expand All @@ -76,7 +76,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception {
.atMost(Duration.ofSeconds(1))
.until(() -> {
providerRepository.setProvider(featureProvider, mockAfterSet(), mockAfterInit(),
mockAfterShutdown(), mockAfterError());
mockAfterShutdown(), mockAfterError(), false);
verify(featureProvider, timeout(TIMEOUT)).initialize(any());
return true;
});
Expand All @@ -101,7 +101,7 @@ class NamedProvider {
@DisplayName("should reject null as named provider")
void shouldRejectNullAsNamedProvider() {
assertThatCode(() -> providerRepository.setProvider(CLIENT_NAME, null, mockAfterSet(), mockAfterInit(),
mockAfterShutdown(), mockAfterError()))
mockAfterShutdown(), mockAfterError(), false))
.isInstanceOf(IllegalArgumentException.class);
}

Expand All @@ -110,7 +110,7 @@ void shouldRejectNullAsNamedProvider() {
void shouldRejectNullAsDefaultProvider() {
NoOpProvider provider = new NoOpProvider();
assertThatCode(() -> providerRepository.setProvider(null, provider, mockAfterSet(), mockAfterInit(),
mockAfterShutdown(), mockAfterError()))
mockAfterShutdown(), mockAfterError(), false))
.isInstanceOf(IllegalArgumentException.class);
}

Expand All @@ -126,7 +126,7 @@ void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() throws Ex
.atMost(Duration.ofSeconds(1))
.until(() -> {
providerRepository.setProvider("named client", featureProvider, mockAfterSet(),
mockAfterInit(), mockAfterShutdown(), mockAfterError());
mockAfterInit(), mockAfterShutdown(), mockAfterError(), false);
verify(featureProvider, timeout(TIMEOUT)).initialize(any());
return true;
});
Expand Down Expand Up @@ -161,7 +161,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception {
.atMost(Duration.ofSeconds(1))
.until(() -> {
providerRepository.setProvider(newProvider, mockAfterSet(), mockAfterInit(),
mockAfterShutdown(), mockAfterError());
mockAfterShutdown(), mockAfterError(), false);
verify(newProvider, timeout(TIMEOUT)).initialize(any());
return true;
});
Expand Down Expand Up @@ -194,7 +194,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception {

Future<?> providerMutation = executorService
.submit(() -> providerRepository.setProvider(CLIENT_NAME, newProvider, mockAfterSet(),
mockAfterInit(), mockAfterShutdown(), mockAfterError()));
mockAfterInit(), mockAfterShutdown(), mockAfterError(), false));

await()
.alias("wait for provider mutator to return")
Expand Down Expand Up @@ -311,7 +311,7 @@ void shouldShutdownAllFeatureProvidersOnShutdown() {

private void setFeatureProvider(FeatureProvider provider) {
providerRepository.setProvider(provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(),
mockAfterError());
mockAfterError(), false);
waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider);
}

Expand All @@ -320,13 +320,13 @@ private void setFeatureProvider(FeatureProvider provider, Consumer<FeatureProvid
Consumer<FeatureProvider> afterInit, Consumer<FeatureProvider> afterShutdown,
BiConsumer<FeatureProvider, String> afterError) {
providerRepository.setProvider(provider, afterSet, afterInit, afterShutdown,
afterError);
afterError, false);
waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider);
}

private void setFeatureProvider(String namedProvider, FeatureProvider provider) {
providerRepository.setProvider(namedProvider, provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(),
mockAfterError());
mockAfterError(), false);
waitForSettingProviderHasBeenCompleted(repository -> repository.getProvider(namedProvider), provider);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,7 @@ public class StepDefinitions {
public static void setup() {
Map<String, Flag<?>> flags = buildFlags();
InMemoryProvider provider = new InMemoryProvider(flags);
OpenFeatureAPI.getInstance().setProvider(provider);

// TODO: setProvider with wait for init, pending https://github.com/open-feature/ofep/pull/80
Thread.sleep(500);

OpenFeatureAPI.getInstance().setProviderAndWait(provider);
client = OpenFeatureAPI.getInstance().getClient();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
import dev.openfeature.sdk.OpenFeatureAPI;
import dev.openfeature.sdk.Value;
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
import dev.openfeature.sdk.exceptions.ProviderNotReadyError;
import dev.openfeature.sdk.exceptions.TypeMismatchError;
import lombok.SneakyThrows;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.omg.CORBA.DynAnyPackage.TypeMismatch;

import java.util.HashMap;
import java.util.Map;

import static dev.openfeature.sdk.Structure.mapToStructure;
Expand All @@ -36,11 +37,7 @@ static void beforeAll() {
Map<String, Flag<?>> flags = buildFlags();
provider = spy(new InMemoryProvider(flags));
OpenFeatureAPI.getInstance().onProviderConfigurationChanged(eventDetails -> {});
OpenFeatureAPI.getInstance().setProvider(provider);

// TODO: setProvider with wait for init, pending https://github.com/open-feature/ofep/pull/80
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
Thread.sleep(500);

OpenFeatureAPI.getInstance().setProviderAndWait(provider);
client = OpenFeatureAPI.getInstance().getClient();
provider.updateFlags(flags);
provider.updateFlag("addedFlag", Flag.builder()
Expand Down Expand Up @@ -99,4 +96,13 @@ void typeMismatch() {
provider.getBooleanEvaluation("string-flag", false, new ImmutableContext());
});
}

@SneakyThrows
@Test
void shouldThrowIfNotInitialized() {
InMemoryProvider inMemoryProvider = new InMemoryProvider(new HashMap<>());

// ErrorCode.PROVIDER_NOT_READY should be returned when evaluated via the client
assertThrows(ProviderNotReadyError.class, ()-> inMemoryProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext()));
}
}
Loading
Loading