Skip to content

Commit 506e89f

Browse files
liran2000toddbaertbeeme1mr
authored
feat: add method to set provider and block during init (#563)
* 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 <[email protected]> * remove method overloading from package private class Signed-off-by: liran2000 <[email protected]> * add test case for spec 2.4.5 Signed-off-by: liran2000 <[email protected]> * minor updates Signed-off-by: liran2000 <[email protected]> --------- Signed-off-by: liran2000 <[email protected]> Co-authored-by: Todd Baert <[email protected]> Co-authored-by: Michael Beemer <[email protected]>
1 parent 7b1eb1c commit 506e89f

File tree

7 files changed

+158
-70
lines changed

7 files changed

+158
-70
lines changed

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

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,12 @@ public EvaluationContext getEvaluationContext() {
100100
public void setProvider(FeatureProvider provider) {
101101
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
102102
providerRepository.setProvider(
103-
provider,
104-
(p) -> attachEventProvider(p),
105-
(p) -> emitReady(p),
106-
(p) -> detachEventProvider(p),
107-
(p, message) -> emitError(p, message));
103+
provider,
104+
this::attachEventProvider,
105+
this::emitReady,
106+
this::detachEventProvider,
107+
this::emitError,
108+
false);
108109
}
109110
}
110111

@@ -117,11 +118,45 @@ public void setProvider(FeatureProvider provider) {
117118
public void setProvider(String clientName, FeatureProvider provider) {
118119
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
119120
providerRepository.setProvider(clientName,
120-
provider,
121-
this::attachEventProvider,
122-
this::emitReady,
123-
this::detachEventProvider,
124-
this::emitError);
121+
provider,
122+
this::attachEventProvider,
123+
this::emitReady,
124+
this::detachEventProvider,
125+
this::emitError,
126+
false);
127+
}
128+
}
129+
130+
/**
131+
* Set the default provider and wait for initialization to finish.
132+
*/
133+
public void setProviderAndWait(FeatureProvider provider) {
134+
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
135+
providerRepository.setProvider(
136+
provider,
137+
this::attachEventProvider,
138+
this::emitReady,
139+
this::detachEventProvider,
140+
this::emitError,
141+
true);
142+
}
143+
}
144+
145+
/**
146+
* Add a provider for a named client and wait for initialization to finish.
147+
*
148+
* @param clientName The name of the client.
149+
* @param provider The provider to set.
150+
*/
151+
public void setProviderAndWait(String clientName, FeatureProvider provider) {
152+
try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
153+
providerRepository.setProvider(clientName,
154+
provider,
155+
this::attachEventProvider,
156+
this::emitReady,
157+
this::detachEventProvider,
158+
this::emitError,
159+
true);
125160
}
126161
}
127162

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

Lines changed: 51 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ public FeatureProvider getProvider(String name) {
4343

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

5050
public Set<String> getAllBoundClientNames() {
@@ -62,58 +62,76 @@ public void setProvider(FeatureProvider provider,
6262
Consumer<FeatureProvider> afterSet,
6363
Consumer<FeatureProvider> afterInit,
6464
Consumer<FeatureProvider> afterShutdown,
65-
BiConsumer<FeatureProvider, String> afterError) {
65+
BiConsumer<FeatureProvider, String> afterError,
66+
boolean waitForInit) {
6667
if (provider == null) {
6768
throw new IllegalArgumentException("Provider cannot be null");
6869
}
69-
initializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError);
70+
prepareAndInitializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit);
7071
}
7172

7273
/**
7374
* Add a provider for a named client.
7475
*
75-
* @param clientName The name of the client.
76-
* @param provider The provider to set.
76+
* @param clientName The name of the client.
77+
* @param provider The provider to set.
78+
* @param waitForInit When true, wait for initialization to finish, then returns.
79+
* Otherwise, initialization happens in the background.
7780
*/
7881
public void setProvider(String clientName,
79-
FeatureProvider provider,
80-
Consumer<FeatureProvider> afterSet,
81-
Consumer<FeatureProvider> afterInit,
82-
Consumer<FeatureProvider> afterShutdown,
83-
BiConsumer<FeatureProvider, String> afterError) {
82+
FeatureProvider provider,
83+
Consumer<FeatureProvider> afterSet,
84+
Consumer<FeatureProvider> afterInit,
85+
Consumer<FeatureProvider> afterShutdown,
86+
BiConsumer<FeatureProvider, String> afterError,
87+
boolean waitForInit) {
8488
if (provider == null) {
8589
throw new IllegalArgumentException("Provider cannot be null");
8690
}
8791
if (clientName == null) {
8892
throw new IllegalArgumentException("clientName cannot be null");
8993
}
90-
initializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError);
94+
prepareAndInitializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit);
9195
}
9296

93-
private void initializeProvider(@Nullable String clientName,
94-
FeatureProvider newProvider,
95-
Consumer<FeatureProvider> afterSet,
96-
Consumer<FeatureProvider> afterInit,
97-
Consumer<FeatureProvider> afterShutdown,
98-
BiConsumer<FeatureProvider, String> afterError) {
97+
private void prepareAndInitializeProvider(@Nullable String clientName,
98+
FeatureProvider newProvider,
99+
Consumer<FeatureProvider> afterSet,
100+
Consumer<FeatureProvider> afterInit,
101+
Consumer<FeatureProvider> afterShutdown,
102+
BiConsumer<FeatureProvider, String> afterError,
103+
boolean waitForInit) {
104+
99105
// provider is set immediately, on this thread
100106
FeatureProvider oldProvider = clientName != null
101-
? this.providers.put(clientName, newProvider)
102-
: this.defaultProvider.getAndSet(newProvider);
107+
? this.providers.put(clientName, newProvider)
108+
: this.defaultProvider.getAndSet(newProvider);
103109
afterSet.accept(newProvider);
104-
taskExecutor.submit(() -> {
105-
// initialization happens in a different thread
106-
try {
107-
if (ProviderState.NOT_READY.equals(newProvider.getState())) {
108-
newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
109-
afterInit.accept(newProvider);
110-
}
111-
shutDownOld(oldProvider, afterShutdown);
112-
} catch (Exception e) {
113-
log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
114-
afterError.accept(newProvider, e.getMessage());
110+
if (waitForInit) {
111+
initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider);
112+
} else {
113+
taskExecutor.submit(() -> {
114+
// initialization happens in a different thread if we're not waiting it
115+
initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider);
116+
});
117+
}
118+
}
119+
120+
private void initializeProvider(FeatureProvider newProvider,
121+
Consumer<FeatureProvider> afterInit,
122+
Consumer<FeatureProvider> afterShutdown,
123+
BiConsumer<FeatureProvider, String> afterError,
124+
FeatureProvider oldProvider) {
125+
try {
126+
if (ProviderState.NOT_READY.equals(newProvider.getState())) {
127+
newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
128+
afterInit.accept(newProvider);
115129
}
116-
});
130+
shutDownOld(oldProvider, afterShutdown);
131+
} catch (Exception e) {
132+
log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e);
133+
afterError.accept(newProvider, e.getMessage());
134+
}
117135
}
118136

119137
private void shutDownOld(FeatureProvider oldProvider,Consumer<FeatureProvider> afterShutdown) {
@@ -157,7 +175,7 @@ public void shutdown() {
157175
},
158176
(FeatureProvider fp,
159177
String message) -> {
160-
});
178+
}, false);
161179
this.providers.clear();
162180
taskExecutor.shutdown();
163181
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717
import java.util.List;
1818
import java.util.Map;
1919

20+
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
21+
import dev.openfeature.sdk.testutils.TestEventsProvider;
22+
import lombok.SneakyThrows;
23+
import org.awaitility.Awaitility;
2024
import org.junit.jupiter.api.AfterEach;
2125
import org.junit.jupiter.api.BeforeEach;
2226
import org.junit.jupiter.api.Test;
@@ -69,6 +73,34 @@ void getApiInstance() {
6973
assertThat(api.getProvider()).isEqualTo(mockProvider);
7074
}
7175

76+
@SneakyThrows
77+
@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.")
78+
@Test void providerAndWait() {
79+
FeatureProvider provider = new TestEventsProvider(500);
80+
OpenFeatureAPI.getInstance().setProviderAndWait(provider);
81+
assertThat(api.getProvider().getState()).isEqualTo(ProviderState.READY);
82+
83+
provider = new TestEventsProvider(500);
84+
String providerName = "providerAndWait";
85+
OpenFeatureAPI.getInstance().setProviderAndWait(providerName, provider);
86+
assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY);
87+
}
88+
89+
@Specification(number="2.4.5", text="The provider SHOULD indicate an error if flag resolution is attempted before the provider is ready.")
90+
@Test void shouldReturnNotReadyIfNotInitialized() {
91+
FeatureProvider provider = new InMemoryProvider(new HashMap<>()) {
92+
@Override
93+
public void initialize(EvaluationContext evaluationContext) throws Exception {
94+
Awaitility.await().wait(3000);
95+
}
96+
};
97+
String providerName = "shouldReturnNotReadyIfNotInitialized";
98+
OpenFeatureAPI.getInstance().setProvider(providerName, provider);
99+
assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.NOT_READY);
100+
Client client = OpenFeatureAPI.getInstance().getClient(providerName);
101+
assertEquals(ErrorCode.PROVIDER_NOT_READY, client.getBooleanDetails("return_error_when_not_initialized", false).getErrorCode());
102+
}
103+
72104
@Specification(number="1.1.5", text="The API MUST provide a function for retrieving the metadata field of the configured provider.")
73105
@Test void provider_metadata() {
74106
FeatureProviderTestUtils.setFeatureProvider(new DoSomethingProvider());
@@ -291,4 +323,5 @@ void getApiInstance() {
291323

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

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class DefaultProvider {
5555
@DisplayName("should reject null as default provider")
5656
void shouldRejectNullAsDefaultProvider() {
5757
assertThatCode(() -> providerRepository.setProvider(null, mockAfterSet(), mockAfterInit(),
58-
mockAfterShutdown(), mockAfterError())).isInstanceOf(IllegalArgumentException.class);
58+
mockAfterShutdown(), mockAfterError(), false)).isInstanceOf(IllegalArgumentException.class);
5959
}
6060

6161
@Test
@@ -76,7 +76,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception {
7676
.atMost(Duration.ofSeconds(1))
7777
.until(() -> {
7878
providerRepository.setProvider(featureProvider, mockAfterSet(), mockAfterInit(),
79-
mockAfterShutdown(), mockAfterError());
79+
mockAfterShutdown(), mockAfterError(), false);
8080
verify(featureProvider, timeout(TIMEOUT)).initialize(any());
8181
return true;
8282
});
@@ -101,7 +101,7 @@ class NamedProvider {
101101
@DisplayName("should reject null as named provider")
102102
void shouldRejectNullAsNamedProvider() {
103103
assertThatCode(() -> providerRepository.setProvider(CLIENT_NAME, null, mockAfterSet(), mockAfterInit(),
104-
mockAfterShutdown(), mockAfterError()))
104+
mockAfterShutdown(), mockAfterError(), false))
105105
.isInstanceOf(IllegalArgumentException.class);
106106
}
107107

@@ -110,7 +110,7 @@ void shouldRejectNullAsNamedProvider() {
110110
void shouldRejectNullAsDefaultProvider() {
111111
NoOpProvider provider = new NoOpProvider();
112112
assertThatCode(() -> providerRepository.setProvider(null, provider, mockAfterSet(), mockAfterInit(),
113-
mockAfterShutdown(), mockAfterError()))
113+
mockAfterShutdown(), mockAfterError(), false))
114114
.isInstanceOf(IllegalArgumentException.class);
115115
}
116116

@@ -126,7 +126,7 @@ void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() throws Ex
126126
.atMost(Duration.ofSeconds(1))
127127
.until(() -> {
128128
providerRepository.setProvider("named client", featureProvider, mockAfterSet(),
129-
mockAfterInit(), mockAfterShutdown(), mockAfterError());
129+
mockAfterInit(), mockAfterShutdown(), mockAfterError(), false);
130130
verify(featureProvider, timeout(TIMEOUT)).initialize(any());
131131
return true;
132132
});
@@ -161,7 +161,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception {
161161
.atMost(Duration.ofSeconds(1))
162162
.until(() -> {
163163
providerRepository.setProvider(newProvider, mockAfterSet(), mockAfterInit(),
164-
mockAfterShutdown(), mockAfterError());
164+
mockAfterShutdown(), mockAfterError(), false);
165165
verify(newProvider, timeout(TIMEOUT)).initialize(any());
166166
return true;
167167
});
@@ -194,7 +194,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception {
194194

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

199199
await()
200200
.alias("wait for provider mutator to return")
@@ -311,7 +311,7 @@ void shouldShutdownAllFeatureProvidersOnShutdown() {
311311

312312
private void setFeatureProvider(FeatureProvider provider) {
313313
providerRepository.setProvider(provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(),
314-
mockAfterError());
314+
mockAfterError(), false);
315315
waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider);
316316
}
317317

@@ -320,13 +320,13 @@ private void setFeatureProvider(FeatureProvider provider, Consumer<FeatureProvid
320320
Consumer<FeatureProvider> afterInit, Consumer<FeatureProvider> afterShutdown,
321321
BiConsumer<FeatureProvider, String> afterError) {
322322
providerRepository.setProvider(provider, afterSet, afterInit, afterShutdown,
323-
afterError);
323+
afterError, false);
324324
waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider);
325325
}
326326

327327
private void setFeatureProvider(String namedProvider, FeatureProvider provider) {
328328
providerRepository.setProvider(namedProvider, provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(),
329-
mockAfterError());
329+
mockAfterError(), false);
330330
waitForSettingProviderHasBeenCompleted(repository -> repository.getProvider(namedProvider), provider);
331331
}
332332

‎src/test/java/dev/openfeature/sdk/e2e/StepDefinitions.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,7 @@ public class StepDefinitions {
5656
public static void setup() {
5757
Map<String, Flag<?>> flags = buildFlags();
5858
InMemoryProvider provider = new InMemoryProvider(flags);
59-
OpenFeatureAPI.getInstance().setProvider(provider);
60-
61-
// TODO: setProvider with wait for init, pending https://github.com/open-feature/ofep/pull/80
62-
Thread.sleep(500);
63-
59+
OpenFeatureAPI.getInstance().setProviderAndWait(provider);
6460
client = OpenFeatureAPI.getInstance().getClient();
6561
}
6662

‎src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@
66
import dev.openfeature.sdk.OpenFeatureAPI;
77
import dev.openfeature.sdk.Value;
88
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
9+
import dev.openfeature.sdk.exceptions.ProviderNotReadyError;
910
import dev.openfeature.sdk.exceptions.TypeMismatchError;
1011
import lombok.SneakyThrows;
1112
import org.junit.jupiter.api.BeforeAll;
1213
import org.junit.jupiter.api.Test;
13-
import org.omg.CORBA.DynAnyPackage.TypeMismatch;
1414

15+
import java.util.HashMap;
1516
import java.util.Map;
1617

1718
import static dev.openfeature.sdk.Structure.mapToStructure;
@@ -36,11 +37,7 @@ static void beforeAll() {
3637
Map<String, Flag<?>> flags = buildFlags();
3738
provider = spy(new InMemoryProvider(flags));
3839
OpenFeatureAPI.getInstance().onProviderConfigurationChanged(eventDetails -> {});
39-
OpenFeatureAPI.getInstance().setProvider(provider);
40-
41-
// TODO: setProvider with wait for init, pending https://github.com/open-feature/ofep/pull/80
42-
Thread.sleep(500);
43-
40+
OpenFeatureAPI.getInstance().setProviderAndWait(provider);
4441
client = OpenFeatureAPI.getInstance().getClient();
4542
provider.updateFlags(flags);
4643
provider.updateFlag("addedFlag", Flag.builder()
@@ -99,4 +96,13 @@ void typeMismatch() {
9996
provider.getBooleanEvaluation("string-flag", false, new ImmutableContext());
10097
});
10198
}
99+
100+
@SneakyThrows
101+
@Test
102+
void shouldThrowIfNotInitialized() {
103+
InMemoryProvider inMemoryProvider = new InMemoryProvider(new HashMap<>());
104+
105+
// ErrorCode.PROVIDER_NOT_READY should be returned when evaluated via the client
106+
assertThrows(ProviderNotReadyError.class, ()-> inMemoryProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext()));
107+
}
102108
}

0 commit comments

Comments
 (0)