Skip to content

Commit 896fe71

Browse files
committed
feat: move factory to isolated package and warn on multi-instance provider binding
1 parent c06e500 commit 896fe71

5 files changed

Lines changed: 81 additions & 5 deletions

File tree

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.List;
1010
import java.util.Optional;
1111
import java.util.Set;
12+
import java.util.concurrent.ConcurrentHashMap;
1213
import java.util.concurrent.ConcurrentLinkedQueue;
1314
import java.util.concurrent.atomic.AtomicReference;
1415
import java.util.function.Consumer;
@@ -22,6 +23,15 @@
2223
@Slf4j
2324
@SuppressWarnings("PMD.UnusedLocalVariable")
2425
public class OpenFeatureAPI implements EventBus<OpenFeatureAPI> {
26+
27+
/**
28+
* Global registry tracking which API instance each provider is currently bound to.
29+
* Used to detect violations of spec requirement 1.8.4 (a provider SHOULD NOT be
30+
* registered with more than one API instance simultaneously).
31+
*/
32+
private static final ConcurrentHashMap<FeatureProvider, OpenFeatureAPI> GLOBAL_PROVIDER_REGISTRY =
33+
new ConcurrentHashMap<>();
34+
2535
// package-private multi-read/single-write lock (instance-level for isolation)
2636
final AutoCloseableReentrantReadWriteLock lock;
2737
private final ConcurrentLinkedQueue<Hook> apiHooks;
@@ -30,7 +40,7 @@ public class OpenFeatureAPI implements EventBus<OpenFeatureAPI> {
3040
private final AtomicReference<EvaluationContext> evaluationContext = new AtomicReference<>();
3141
private TransactionContextPropagator transactionContextPropagator;
3242

33-
protected OpenFeatureAPI() {
43+
public OpenFeatureAPI() {
3444
this(new AutoCloseableReentrantReadWriteLock());
3545
}
3646

@@ -338,6 +348,31 @@ public void clearHooks() {
338348
this.apiHooks.clear();
339349
}
340350

351+
/**
352+
* Registers a provider with the global registry, warning if it is already
353+
* bound to a different API instance (spec requirement 1.8.4).
354+
*/
355+
void registerGlobalProvider(FeatureProvider provider) {
356+
GLOBAL_PROVIDER_REGISTRY.compute(provider, (p, existing) -> {
357+
if (existing != null && existing != this) {
358+
log.warn(
359+
"Provider "
360+
+ provider.getClass().getName()
361+
+ " is already registered with another API instance. "
362+
+ "A provider SHOULD NOT be bound to more than one API instance "
363+
+ "simultaneously (spec requirement 1.8.4).");
364+
}
365+
return this;
366+
});
367+
}
368+
369+
/**
370+
* Removes the provider from the global registry if this instance is the current owner.
371+
*/
372+
void deregisterGlobalProvider(FeatureProvider provider) {
373+
GLOBAL_PROVIDER_REGISTRY.remove(provider, this);
374+
}
375+
341376
/**
342377
* Shut down and reset the current status of OpenFeature API.
343378
* This call cleans up all active providers and attempts to shut down internal

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,8 @@ private void prepareAndInitializeProvider(
173173
newStateManager = new FeatureProviderStateManager(newProvider);
174174
// only run afterSet if new provider is not already attached
175175
afterSet.accept(newProvider);
176+
// spec 1.8.4: warn if this provider is already bound to another API instance
177+
openFeatureAPI.registerGlobalProvider(newProvider);
176178
} else {
177179
newStateManager = existing;
178180
}
@@ -236,6 +238,8 @@ private void initializeProvider(
236238
private void shutDownOld(FeatureProviderStateManager oldManager, Consumer<FeatureProvider> afterShutdown) {
237239
synchronized (registerStateManagerLock) {
238240
if (oldManager != null && !isStateManagerRegistered(oldManager)) {
241+
// spec 1.8.4: release the provider from the global registry
242+
openFeatureAPI.deregisterGlobalProvider(oldManager.getProvider());
239243
shutdownProvider(oldManager);
240244
afterShutdown.accept(oldManager.getProvider());
241245
}
@@ -327,7 +331,11 @@ List<FeatureProviderStateManager> prepareShutdown() {
327331
* @param managersToShutdown the managers to shut down (from prepareShutdown)
328332
*/
329333
void completeShutdown(List<FeatureProviderStateManager> managersToShutdown) {
330-
managersToShutdown.forEach(this::shutdownProvider);
334+
managersToShutdown.forEach(m -> {
335+
// spec 1.8.4: release all providers from the global registry on shutdown
336+
openFeatureAPI.deregisterGlobalProvider(m.getProvider());
337+
shutdownProvider(m);
338+
});
331339
taskExecutor.shutdown();
332340
try {
333341
if (!taskExecutor.awaitTermination(EventSupport.SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {

src/main/java/dev/openfeature/sdk/OpenFeatureAPIFactory.java renamed to src/main/java/dev/openfeature/sdk/isolated/OpenFeatureAPIFactory.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
package dev.openfeature.sdk;
1+
package dev.openfeature.sdk.isolated;
2+
3+
import dev.openfeature.sdk.OpenFeatureAPI;
24

35
/**
46
* Factory for creating isolated OpenFeature API instances.
@@ -8,12 +10,18 @@
810
* transaction context propagators. Instances do not share state with the
911
* global singleton ({@link OpenFeatureAPI#getInstance()}) or with each other.
1012
*
13+
* <p>This class lives in a distinct package ({@code dev.openfeature.sdk.isolated})
14+
* to make isolated instances intentionally less discoverable than the global
15+
* singleton, reducing the chance of accidental use when the singleton would be
16+
* appropriate.
17+
*
1118
* <p>This is useful for dependency injection frameworks, testing scenarios,
1219
* and applications composed of multiple submodules requiring distinct providers.
1320
*
1421
* <p><strong>Spec references:</strong>
1522
* <ul>
1623
* <li>Requirement 1.8.1 &mdash; factory function for isolated instances</li>
24+
* <li>Requirement 1.8.3 &mdash; factory in a distinct package/module</li>
1725
* </ul>
1826
*
1927
* @see <a href="https://openfeature.dev/specification/sections/flag-evaluation#18-isolated-api-instances">

src/test/java/dev/openfeature/sdk/isolated/IsolatedAPISingeltonTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import dev.openfeature.sdk.ImmutableContext;
77
import dev.openfeature.sdk.NoOpTransactionContextPropagator;
88
import dev.openfeature.sdk.OpenFeatureAPI;
9-
import dev.openfeature.sdk.OpenFeatureAPIFactory;
109
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
1110
import java.util.Map;
1211
import org.junit.jupiter.api.AfterEach;

src/test/java/dev/openfeature/sdk/isolated/IsolatedAPITest.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
package dev.openfeature.sdk.isolated;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.mockito.ArgumentMatchers.contains;
45

56
import dev.openfeature.sdk.ImmutableContext;
67
import dev.openfeature.sdk.NoOpProvider;
78
import dev.openfeature.sdk.NoOpTransactionContextPropagator;
89
import dev.openfeature.sdk.OpenFeatureAPI;
9-
import dev.openfeature.sdk.OpenFeatureAPIFactory;
1010
import dev.openfeature.sdk.ThreadLocalTransactionContextPropagator;
11+
import org.mockito.Mockito;
12+
import org.simplify4u.slf4jmock.LoggerMock;
13+
import org.slf4j.Logger;
1114
import dev.openfeature.sdk.providers.memory.Flag;
1215
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
1316
import java.util.Map;
@@ -201,4 +204,27 @@ void clientUsesItsOwnInstanceProvider() throws Exception {
201204
// api2 has NoOpProvider, so it returns the default
202205
assertThat(client2.getBooleanValue("flag1", false)).isFalse();
203206
}
207+
208+
/**
209+
* Requirement 1.8.4 — a warning is logged when the same provider instance
210+
* is registered with more than one API instance simultaneously.
211+
*/
212+
@Test
213+
@DisplayName("warn when same provider bound to multiple API instances (req 1.8.4)")
214+
void warnWhenProviderBoundToMultipleInstances() {
215+
Logger mockLogger = Mockito.mock(Logger.class);
216+
LoggerMock.setMock(OpenFeatureAPI.class, mockLogger);
217+
try {
218+
OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI();
219+
OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI();
220+
221+
NoOpProvider provider = new NoOpProvider();
222+
api1.setProvider(provider);
223+
api2.setProvider(provider);
224+
225+
Mockito.verify(mockLogger).warn(contains("1.8.4"));
226+
} finally {
227+
LoggerMock.setMock(OpenFeatureAPI.class, null);
228+
}
229+
}
204230
}

0 commit comments

Comments
 (0)