Skip to content

Commit 4437b89

Browse files
lopitzlopitz
committed
- addresses review comments
Signed-off-by: Lars Opitz <[email protected]>
1 parent c09b814 commit 4437b89

3 files changed

Lines changed: 63 additions & 17 deletions

File tree

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ public class OpenFeatureAPI {
1919
static AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock();
2020
static AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock();
2121

22-
private final ProviderRepository providerRepository = new ProviderRepository();
2322
private final List<Hook> apiHooks;
2423

24+
private ProviderRepository providerRepository = new ProviderRepository();
2525
private EvaluationContext evaluationContext;
2626

27-
private OpenFeatureAPI() {
27+
protected OpenFeatureAPI() {
2828
this.apiHooks = new ArrayList<>();
2929
}
3030

@@ -139,4 +139,15 @@ public void clearHooks() {
139139
this.apiHooks.clear();
140140
}
141141
}
142+
143+
public void shutdown() {
144+
providerRepository.shutdown();
145+
}
146+
147+
/**
148+
* This method is only here for testing as otherwise all tests after the API shutdown test would fail.
149+
*/
150+
final void resetProviderRepository() {
151+
providerRepository = new ProviderRepository();
152+
}
142153
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ private void initializeProvider(String clientName, FeatureProvider provider) {
7676
private void initializeProvider(FeatureProvider provider, Consumer<FeatureProvider> afterInitialization) {
7777
taskExecutor.submit(() -> {
7878
try {
79-
if (isProviderNotBoundMultipleTimes(provider)) {
79+
if (!isProviderRegistered(provider)) {
8080
provider.initialize();
8181
}
8282
afterInitialization.accept(provider);
@@ -121,13 +121,13 @@ private void updateNamedProviderAfterInitialization(String clientName, FeaturePr
121121
private void replaceNamedProviderAndShutdownOldOne(String clientName, FeatureProvider provider) {
122122
FeatureProvider oldProvider = this.providers.put(clientName, provider);
123123
this.initializingNamedProviders.remove(clientName, provider);
124-
if (isProviderNotBoundMultipleTimes(oldProvider)) {
124+
if (!isProviderRegistered(oldProvider)) {
125125
shutdownProvider(oldProvider);
126126
}
127127
}
128128

129-
private boolean isProviderNotBoundMultipleTimes(FeatureProvider oldProvider) {
130-
return !(this.providers.containsValue(oldProvider) || this.defaultProvider.get().equals(oldProvider));
129+
private boolean isProviderRegistered(FeatureProvider oldProvider) {
130+
return this.providers.containsValue(oldProvider) || this.defaultProvider.get().equals(oldProvider);
131131
}
132132

133133
private void shutdownProvider(FeatureProvider provider) {

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

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
package dev.openfeature.sdk;
22

3+
import dev.openfeature.sdk.fixtures.ProviderFixture;
34
import dev.openfeature.sdk.testutils.exception.TestException;
4-
import org.junit.jupiter.api.*;
5+
import org.awaitility.Awaitility;
6+
import org.junit.jupiter.api.BeforeEach;
7+
import org.junit.jupiter.api.DisplayName;
8+
import org.junit.jupiter.api.Nested;
9+
import org.junit.jupiter.api.Test;
10+
11+
import java.time.Duration;
512

613
import static dev.openfeature.sdk.testutils.FeatureProviderTestUtils.setFeatureProvider;
714
import static org.mockito.Mockito.*;
@@ -20,7 +27,7 @@ class DefaultProvider {
2027
@Test
2128
@DisplayName("must invoke shutdown method on previously registered provider once it should not be used for flag evaluation anymore")
2229
void mustInvokeShutdownMethodOnPreviouslyRegisteredProviderOnceItShouldNotBeUsedForFlagEvaluationAnymore() {
23-
FeatureProvider featureProvider = mock(FeatureProvider.class);
30+
FeatureProvider featureProvider = ProviderFixture.createMockedProvider();
2431

2532
setFeatureProvider(featureProvider);
2633
setFeatureProvider(new NoOpProvider());
@@ -29,13 +36,13 @@ void mustInvokeShutdownMethodOnPreviouslyRegisteredProviderOnceItShouldNotBeUsed
2936
}
3037

3138
@Specification(number = "1.4.9", text = "Methods, functions, or operations on the client MUST NOT throw "
32-
+ "exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the "
33-
+ "`default value` in the event of abnormal execution. Exceptions include functions or methods for "
34-
+ "the purposes for configuration or setup.")
39+
+ "exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the "
40+
+ "`default value` in the event of abnormal execution. Exceptions include functions or methods for "
41+
+ "the purposes for configuration or setup.")
3542
@Test
3643
@DisplayName("should catch exception thrown by the provider on shutdown")
3744
void shouldCatchExceptionThrownByTheProviderOnShutdown() {
38-
FeatureProvider featureProvider = mock(FeatureProvider.class);
45+
FeatureProvider featureProvider = ProviderFixture.createMockedProvider();
3946
doThrow(TestException.class).when(featureProvider).shutdown();
4047

4148
setFeatureProvider(featureProvider);
@@ -53,7 +60,7 @@ class NamedProvider {
5360
@DisplayName("must invoke shutdown method on previously registered provider once it should not be used for flag evaluation anymore")
5461
void mustInvokeShutdownMethodOnPreviouslyRegisteredProviderOnceItShouldNotBeUsedForFlagEvaluationAnymore() {
5562
String clientName = "clientName";
56-
FeatureProvider featureProvider = mock(FeatureProvider.class);
63+
FeatureProvider featureProvider = ProviderFixture.createMockedProvider();
5764

5865
setFeatureProvider(clientName, featureProvider);
5966
setFeatureProvider(clientName, new NoOpProvider());
@@ -62,21 +69,49 @@ void mustInvokeShutdownMethodOnPreviouslyRegisteredProviderOnceItShouldNotBeUsed
6269
}
6370

6471
@Specification(number = "1.4.9", text = "Methods, functions, or operations on the client MUST NOT throw "
65-
+ "exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the "
66-
+ "`default value` in the event of abnormal execution. Exceptions include functions or methods for "
67-
+ "the purposes for configuration or setup.")
72+
+ "exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the "
73+
+ "`default value` in the event of abnormal execution. Exceptions include functions or methods for "
74+
+ "the purposes for configuration or setup.")
6875
@Test
6976
@DisplayName("should catch exception thrown by the named client provider on shutdown")
7077
void shouldCatchExceptionThrownByTheNamedClientProviderOnShutdown() {
7178
String clientName = "clientName";
72-
FeatureProvider featureProvider = mock(FeatureProvider.class);
79+
FeatureProvider featureProvider = ProviderFixture.createMockedProvider();
7380
doThrow(TestException.class).when(featureProvider).shutdown();
7481

7582
setFeatureProvider(clientName, featureProvider);
7683
setFeatureProvider(clientName, new NoOpProvider());
7784

7885
verify(featureProvider, timeout(1000)).shutdown();
7986
}
87+
}
88+
89+
@Nested
90+
class General {
8091

92+
@Specification(number = "1.6.1", text = "The API MUST define a shutdown function which, when called, must call the respective shutdown function on the active provider.")
93+
@Test
94+
@DisplayName("must shutdown all providers on shutting down api")
95+
void mustShutdownAllProvidersOnShuttingDownApi() {
96+
FeatureProvider defaultProvider = ProviderFixture.createMockedProvider();
97+
FeatureProvider namedProvider = ProviderFixture.createMockedProvider();
98+
setFeatureProvider(defaultProvider);
99+
setFeatureProvider("clientName", namedProvider);
100+
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
101+
102+
synchronized (OpenFeatureAPI.class) {
103+
api.shutdown();
104+
105+
Awaitility
106+
.await()
107+
.atMost(Duration.ofSeconds(1))
108+
.untilAsserted(() -> {
109+
verify(defaultProvider).shutdown();
110+
verify(namedProvider).shutdown();
111+
});
112+
113+
api.resetProviderRepository();
114+
}
115+
}
81116
}
82117
}

0 commit comments

Comments
 (0)