Skip to content
42 changes: 31 additions & 11 deletions src/main/java/dev/openfeature/sdk/EventProvider.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package dev.openfeature.sdk;

import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
import dev.openfeature.sdk.internal.ConfigurableThreadFactory;
import dev.openfeature.sdk.internal.TriConsumer;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -29,29 +30,46 @@ void setEventProviderListener(EventProviderListener eventProviderListener) {
this.eventProviderListener = eventProviderListener;
}

private TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit = null;
// Bundles onEmit and lock into a single volatile reference so they are always read atomically:
// a non-null attachment guarantees a non-null lock.
private static final class Attachment {
final TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit;
final AutoCloseableReentrantReadWriteLock lock;

Attachment(
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit,
AutoCloseableReentrantReadWriteLock lock) {
this.onEmit = onEmit;
this.lock = lock;
}
}

private volatile Attachment attachment = null;

/**
* "Attach" this EventProvider to an SDK, which allows events to propagate from this provider.
* No-op if the same onEmit is already attached.
*
* @param onEmit the function to run when a provider emits events.
* @param lock the API instance's read/write lock for thread safety.
* @throws IllegalStateException if attempted to bind a new emitter for already bound provider
*/
void attach(TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit) {
if (this.onEmit != null && this.onEmit != onEmit) {
void attach(
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit,
AutoCloseableReentrantReadWriteLock lock) {
Attachment existing = this.attachment;
if (existing != null && existing.onEmit != onEmit) {
// if we are trying to attach this provider to a different onEmit, something has gone wrong
throw new IllegalStateException("Provider " + this.getMetadata().getName() + " is already attached.");
} else {
this.onEmit = onEmit;
}
this.attachment = new Attachment(onEmit, lock);
}

/**
* "Detach" this EventProvider from an SDK, stopping propagation of all events.
*/
void detach() {
this.onEmit = null;
this.attachment = null;
}

/**
Expand Down Expand Up @@ -80,9 +98,9 @@ public void shutdown() {
*/
public Awaitable emit(final ProviderEvent event, final ProviderEventDetails details) {
final var localEventProviderListener = this.eventProviderListener;
final var localOnEmit = this.onEmit;
final var localAttachment = this.attachment;

if (localEventProviderListener == null && localOnEmit == null) {
if (localEventProviderListener == null && localAttachment == null) {
return Awaitable.FINISHED;
}

Expand All @@ -91,12 +109,14 @@ public Awaitable emit(final ProviderEvent event, final ProviderEventDetails deta
// These calls need to be executed on a different thread to prevent deadlocks when the provider initialization
// relies on a ready event to be emitted
emitterExecutor.submit(() -> {
try (var ignored = OpenFeatureAPI.lock.readLockAutoCloseable()) {
// Lock is only needed when attached to an API instance. A non-null attachment always
// carries a non-null lock, so no null check on the lock itself is required.
try (var ignored = localAttachment != null ? localAttachment.lock.readLockAutoCloseable() : null) {
if (localEventProviderListener != null) {
localEventProviderListener.onEmit(event, details);
}
if (localOnEmit != null) {
localOnEmit.accept(this, event, details);
if (localAttachment != null) {
localAttachment.onEmit.accept(this, event, details);
}
} finally {
awaitable.wakeup();
Expand Down
48 changes: 44 additions & 4 deletions src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
Expand All @@ -22,15 +23,30 @@
@Slf4j
@SuppressWarnings("PMD.UnusedLocalVariable")
public class OpenFeatureAPI implements EventBus<OpenFeatureAPI> {
// package-private multi-read/single-write lock
static AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock();

/**
* Global registry tracking which API instance each provider is currently bound to.
* Used to detect violations of spec requirement 1.8.4 (a provider SHOULD NOT be
* registered with more than one API instance simultaneously).
*/
private static final ConcurrentHashMap<FeatureProvider, OpenFeatureAPI> GLOBAL_PROVIDER_REGISTRY =
new ConcurrentHashMap<>();

// package-private multi-read/single-write lock (instance-level for isolation)
final AutoCloseableReentrantReadWriteLock lock;
private final ConcurrentLinkedQueue<Hook> apiHooks;
private ProviderRepository providerRepository;
private EventSupport eventSupport;
private final AtomicReference<EvaluationContext> evaluationContext = new AtomicReference<>();
private TransactionContextPropagator transactionContextPropagator;

protected OpenFeatureAPI() {
public OpenFeatureAPI() {
this(new AutoCloseableReentrantReadWriteLock());
}

// Package-private constructor for testing with a custom lock.
OpenFeatureAPI(AutoCloseableReentrantReadWriteLock lock) {
this.lock = lock;
apiHooks = new ConcurrentLinkedQueue<>();
providerRepository = new ProviderRepository(this);
eventSupport = new EventSupport();
Expand Down Expand Up @@ -251,7 +267,7 @@ public void setProviderAndWait(String domain, FeatureProvider provider) throws O

private void attachEventProvider(FeatureProvider provider) {
if (provider instanceof EventProvider) {
((EventProvider) provider).attach(this::runHandlersForProvider);
((EventProvider) provider).attach(this::runHandlersForProvider, this.lock);
}
}

Expand Down Expand Up @@ -332,6 +348,30 @@ public void clearHooks() {
this.apiHooks.clear();
}

/**
* Registers a provider with the global registry, warning if it is already
* bound to a different API instance (spec requirement 1.8.4).
*/
void registerGlobalProvider(FeatureProvider provider) {
GLOBAL_PROVIDER_REGISTRY.compute(provider, (p, existing) -> {
if (existing != null && existing != this) {
log.warn("Provider "
+ provider.getClass().getName()
+ " is already registered with another API instance. "
+ "A provider SHOULD NOT be bound to more than one API instance "
+ "simultaneously (spec requirement 1.8.4).");
}
return this;
});
}

/**
* Removes the provider from the global registry if this instance is the current owner.
*/
void deregisterGlobalProvider(FeatureProvider provider) {
GLOBAL_PROVIDER_REGISTRY.remove(provider, this);
}

/**
* Shut down and reset the current status of OpenFeature API.
* This call cleans up all active providers and attempts to shut down internal
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/dev/openfeature/sdk/ProviderRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ private void prepareAndInitializeProvider(
newStateManager = new FeatureProviderStateManager(newProvider);
// only run afterSet if new provider is not already attached
afterSet.accept(newProvider);
// spec 1.8.4: warn if this provider is already bound to another API instance
openFeatureAPI.registerGlobalProvider(newProvider);
} else {
newStateManager = existing;
}
Expand Down Expand Up @@ -236,6 +238,8 @@ private void initializeProvider(
private void shutDownOld(FeatureProviderStateManager oldManager, Consumer<FeatureProvider> afterShutdown) {
synchronized (registerStateManagerLock) {
if (oldManager != null && !isStateManagerRegistered(oldManager)) {
// spec 1.8.4: release the provider from the global registry
openFeatureAPI.deregisterGlobalProvider(oldManager.getProvider());
shutdownProvider(oldManager);
afterShutdown.accept(oldManager.getProvider());
}
Expand Down Expand Up @@ -327,7 +331,11 @@ List<FeatureProviderStateManager> prepareShutdown() {
* @param managersToShutdown the managers to shut down (from prepareShutdown)
*/
void completeShutdown(List<FeatureProviderStateManager> managersToShutdown) {
managersToShutdown.forEach(this::shutdownProvider);
managersToShutdown.forEach(m -> {
// spec 1.8.4: release all providers from the global registry on shutdown
openFeatureAPI.deregisterGlobalProvider(m.getProvider());
shutdownProvider(m);
});
taskExecutor.shutdown();
try {
if (!taskExecutor.awaitTermination(EventSupport.SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package dev.openfeature.sdk.isolated;

import dev.openfeature.sdk.OpenFeatureAPI;

/**
* Factory for creating isolated OpenFeature API instances.
*
* <p>Each instance returned by {@link #createAPI()} maintains its own state,
* including providers, evaluation context, hooks, event handlers, and
* transaction context propagators. Instances do not share state with the
* global singleton ({@link OpenFeatureAPI#getInstance()}) or with each other.
*
* <p>This class lives in a distinct package ({@code dev.openfeature.sdk.isolated})
* to make isolated instances intentionally less discoverable than the global
* singleton, reducing the chance of accidental use when the singleton would be
* appropriate.
*
* <p>This is useful for dependency injection frameworks, testing scenarios,
* and applications composed of multiple submodules requiring distinct providers.
*
* <p><strong>Spec references:</strong>
* <ul>
* <li>Requirement 1.8.1 &mdash; factory function for isolated instances</li>
* <li>Requirement 1.8.3 &mdash; factory in a distinct package/module</li>
* </ul>
*
* @see <a href="https://openfeature.dev/specification/sections/flag-evaluation#18-isolated-api-instances">
* Spec &sect;1.8 &mdash; Isolated API Instances</a>
*/
public final class OpenFeatureAPIFactory {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Section 1.8 is marked experimental in the spec. Might be worth adding an @apiNote to flag that, e.g.:

* @apiNote This API is experimental and subject to change.

Could go on the class-level Javadoc or on createAPI() itself (or both).


private OpenFeatureAPIFactory() {
// utility class
}

/**
* Creates a new, independent {@link OpenFeatureAPI} instance with fully
* isolated state.
*
* <p>Usage:
* <pre>{@code
* OpenFeatureAPI api = OpenFeatureAPIFactory.createAPI();
* api.setProvider(new MyProvider());
* Client client = api.getClient();
* }</pre>
*
* @return a new API instance
*/
public static OpenFeatureAPI createAPI() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully see the point of this method/class. OpenFeatureAPI.createIsolated() has the exact same visibility level and does the same thing. I think this will only lead to confusion

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

return new OpenFeatureAPI();
}
}
19 changes: 12 additions & 7 deletions src/test/java/dev/openfeature/sdk/EventProviderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;

import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
import dev.openfeature.sdk.internal.TriConsumer;
import dev.openfeature.sdk.testutils.TestStackedEmitCallsProvider;
import io.cucumber.java.AfterAll;
Expand Down Expand Up @@ -36,7 +37,7 @@ public static void resetDefaultProvider() {
@DisplayName("should run attached onEmit with emitters")
void emitsEventsWhenAttached() {
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit = mockOnEmit();
eventProvider.attach(onEmit);
eventProvider.attach(onEmit, new AutoCloseableReentrantReadWriteLock());

ProviderEventDetails details = ProviderEventDetails.builder().build();
eventProvider.emit(ProviderEvent.PROVIDER_READY, details);
Expand Down Expand Up @@ -73,17 +74,19 @@ void doesNotEmitsEventsWhenNotAttached() {
void throwsWhenOnEmitDifferent() {
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit1 = mockOnEmit();
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit2 = mockOnEmit();
eventProvider.attach(onEmit1);
assertThrows(IllegalStateException.class, () -> eventProvider.attach(onEmit2));
AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock();
eventProvider.attach(onEmit1, lock);
assertThrows(IllegalStateException.class, () -> eventProvider.attach(onEmit2, lock));
}

@Test
@DisplayName("should not throw if second same onEmit attached")
void doesNotThrowWhenOnEmitSame() {
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit1 = mockOnEmit();
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit2 = onEmit1;
eventProvider.attach(onEmit1);
eventProvider.attach(onEmit2); // should not throw, same instance. noop
eventProvider.attach(onEmit1, new AutoCloseableReentrantReadWriteLock());
eventProvider.attach(
onEmit2, new AutoCloseableReentrantReadWriteLock()); // should not throw, same instance. noop
}

@Test
Expand Down Expand Up @@ -132,8 +135,10 @@ public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultVa
}

@Override
public void attach(TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit) {
super.attach(onEmit);
public void attach(
TriConsumer<EventProvider, ProviderEvent, ProviderEventDetails> onEmit,
AutoCloseableReentrantReadWriteLock lock) {
super.attach(onEmit, lock);
}
}

Expand Down
14 changes: 3 additions & 11 deletions src/test/java/dev/openfeature/sdk/LockingSingeltonTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.function.Consumer;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
Expand All @@ -17,23 +16,16 @@
@Isolated()
class LockingSingeltonTest {

private static OpenFeatureAPI api;
private OpenFeatureAPI api;
private OpenFeatureClient client;
private AutoCloseableReentrantReadWriteLock apiLock;
private AutoCloseableReentrantReadWriteLock clientHooksLock;

@BeforeAll
static void beforeAll() {
api = OpenFeatureAPI.getInstance();
OpenFeatureAPI.getInstance().setProvider("LockingTest", new NoOpProvider());
}

@BeforeEach
void beforeEach() {
client = (OpenFeatureClient) api.getClient("LockingTest");

apiLock = setupLock(apiLock, mockInnerReadLock(), mockInnerWriteLock());
OpenFeatureAPI.lock = apiLock;
api = new OpenFeatureAPI(apiLock);
client = (OpenFeatureClient) api.getClient("LockingTest");

clientHooksLock = setupLock(clientHooksLock, mockInnerReadLock(), mockInnerWriteLock());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package dev.openfeature.sdk.isolated;

import static org.assertj.core.api.Assertions.assertThat;

import dev.openfeature.sdk.FeatureProvider;
import dev.openfeature.sdk.ImmutableContext;
import dev.openfeature.sdk.NoOpTransactionContextPropagator;
import dev.openfeature.sdk.OpenFeatureAPI;
import dev.openfeature.sdk.providers.memory.InMemoryProvider;
import java.util.Map;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

class IsolatedAPISingeltonTest {

private final OpenFeatureAPI singleton = OpenFeatureAPI.getInstance();

@AfterEach
void restoreSingleton() {
singleton.shutdown();
singleton.clearHooks();
singleton.setEvaluationContext(null);
singleton.setTransactionContextPropagator(new NoOpTransactionContextPropagator());
}

/**
* Requirement 1.8.1 — isolated instances do not share state with
* the global singleton.
*/
@Test
@DisplayName("isolated instance does not interfere with singleton")
void isolatedInstanceDoesNotInterfereWithSingleton() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests that mutating an isolated instance doesn't affect the singleton, which is great. Worth adding the reverse direction too; e.g. set a provider/hook/context on the singleton and assert a previously created isolated instance is unaffected.

// record singleton baseline
FeatureProvider singletonProvider = singleton.getProvider();

OpenFeatureAPI isolated = OpenFeatureAPIFactory.createAPI();
assertThat(isolated).isNotSameAs(singleton);

// mutate only the isolated instance
isolated.setProvider(new InMemoryProvider(Map.of()));
isolated.addHooks(new NoOpHook());
isolated.setEvaluationContext(new ImmutableContext("isolated-key"));

// singleton remains at baseline
assertThat(singleton.getProvider()).isSameAs(singletonProvider);
assertThat(singleton.getHooks()).isEmpty();
assertThat(singleton.getEvaluationContext()).isNull();
}
}
Loading
Loading