Skip to content

Commit

Permalink
feat: add method to set provider and block during init (#563)
Browse files Browse the repository at this point in the history 
* feat: spec 1.1.8 - setProviderAndWait

The API SHOULD provide functions to set a provider and wait for the initialize function to return or throw

Signed-off-by: liran2000 <liran2000@gmail.com>

* remove method overloading from package private class

Signed-off-by: liran2000 <liran2000@gmail.com>

* add test case for spec 2.4.5

Signed-off-by: liran2000 <liran2000@gmail.com>

* minor updates

Signed-off-by: liran2000 <liran2000@gmail.com>

---------

Signed-off-by: liran2000 <liran2000@gmail.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
  • Loading branch information
@liran2000 @toddbaert @beeme1mr
3 people authored Aug 28, 2023
1 parent 7b1eb1c commit 506e89f
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 70 deletions.
55 changes: 45 additions & 10 deletions src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,12 @@ public EvaluationContext getEvaluationContext() {
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,
this::attachEventProvider,
this::emitReady,
this::detachEventProvider,
this::emitError,
false);
}
}

Expand All @@ -117,11 +118,45 @@ public void setProvider(FeatureProvider provider) {
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,
this::attachEventProvider,
this::emitReady,
this::detachEventProvider,
this::emitError,
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
View file
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());
}
}

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
View file
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
View file
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
6 changes: 1 addition & 5 deletions src/test/java/dev/openfeature/sdk/e2e/StepDefinitions.java
View file
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
18 changes: 12 additions & 6 deletions src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java
View file
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
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

0 comments on commit 506e89f

Please sign in to comment.