diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index c126c1451..08d84040d 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -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; @@ -29,29 +30,46 @@ void setEventProviderListener(EventProviderListener eventProviderListener) { this.eventProviderListener = eventProviderListener; } - private TriConsumer 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 onEmit; + final AutoCloseableReentrantReadWriteLock lock; + + Attachment( + TriConsumer 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 onEmit) { - if (this.onEmit != null && this.onEmit != onEmit) { + void attach( + TriConsumer 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; } /** @@ -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; } @@ -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(); diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 02c1edf25..cf3cf0d80 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -22,8 +22,8 @@ @Slf4j @SuppressWarnings("PMD.UnusedLocalVariable") public class OpenFeatureAPI implements EventBus { - // package-private multi-read/single-write lock - static AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock(); + // package-private multi-read/single-write lock (instance-level for isolation) + final AutoCloseableReentrantReadWriteLock lock; private final ConcurrentLinkedQueue apiHooks; private ProviderRepository providerRepository; private EventSupport eventSupport; @@ -31,6 +31,12 @@ public class OpenFeatureAPI implements EventBus { private TransactionContextPropagator transactionContextPropagator; protected 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(); @@ -251,7 +257,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); } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPIFactory.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPIFactory.java new file mode 100644 index 000000000..283876932 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPIFactory.java @@ -0,0 +1,44 @@ +package dev.openfeature.sdk; + +/** + * Factory for creating isolated OpenFeature API instances. + * + *

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. + * + *

This is useful for dependency injection frameworks, testing scenarios, + * and applications composed of multiple submodules requiring distinct providers. + * + *

Spec references: + *

    + *
  • Requirement 1.8.1 — factory function for isolated instances
    • + *
+ * + * @see + * Spec §1.8 — Isolated API Instances + */ +public final class OpenFeatureAPIFactory { + + private OpenFeatureAPIFactory() { + // utility class + } + + /** + * Creates a new, independent {@link OpenFeatureAPI} instance with fully + * isolated state. + * + *

Usage: + * 

{@code
+     * OpenFeatureAPI api = OpenFeatureAPIFactory.createAPI();
+     * api.setProvider(new MyProvider());
+     * Client client = api.getClient();
+     * }
+ * + * @return a new API instance + */ + public static OpenFeatureAPI createAPI() { + return new OpenFeatureAPI(); + } +} diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index d04fa88d1..253a0329c 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -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; @@ -36,7 +37,7 @@ public static void resetDefaultProvider() { @DisplayName("should run attached onEmit with emitters") void emitsEventsWhenAttached() { TriConsumer onEmit = mockOnEmit(); - eventProvider.attach(onEmit); + eventProvider.attach(onEmit, new AutoCloseableReentrantReadWriteLock()); ProviderEventDetails details = ProviderEventDetails.builder().build(); eventProvider.emit(ProviderEvent.PROVIDER_READY, details); @@ -73,8 +74,9 @@ void doesNotEmitsEventsWhenNotAttached() { void throwsWhenOnEmitDifferent() { TriConsumer onEmit1 = mockOnEmit(); TriConsumer 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 @@ -82,8 +84,9 @@ void throwsWhenOnEmitDifferent() { void doesNotThrowWhenOnEmitSame() { TriConsumer onEmit1 = mockOnEmit(); TriConsumer 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 @@ -132,8 +135,10 @@ public ProviderEvaluation getObjectEvaluation(String key, Value defaultVa } @Override - public void attach(TriConsumer onEmit) { - super.attach(onEmit); + public void attach( + TriConsumer onEmit, + AutoCloseableReentrantReadWriteLock lock) { + super.attach(onEmit, lock); } } diff --git a/src/test/java/dev/openfeature/sdk/LockingSingeltonTest.java b/src/test/java/dev/openfeature/sdk/LockingSingeltonTest.java index ae3246cae..b1fc0a6b2 100644 --- a/src/test/java/dev/openfeature/sdk/LockingSingeltonTest.java +++ b/src/test/java/dev/openfeature/sdk/LockingSingeltonTest.java @@ -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; @@ -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()); } diff --git a/src/test/java/dev/openfeature/sdk/isolated/IsolatedAPISingeltonTest.java b/src/test/java/dev/openfeature/sdk/isolated/IsolatedAPISingeltonTest.java new file mode 100644 index 000000000..7699d4910 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/isolated/IsolatedAPISingeltonTest.java @@ -0,0 +1,51 @@ +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.OpenFeatureAPIFactory; +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() { + // 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(); + } +} diff --git a/src/test/java/dev/openfeature/sdk/isolated/IsolatedAPITest.java b/src/test/java/dev/openfeature/sdk/isolated/IsolatedAPITest.java new file mode 100644 index 000000000..2a673a95d --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/isolated/IsolatedAPITest.java @@ -0,0 +1,204 @@ +package dev.openfeature.sdk.isolated; + +import static org.assertj.core.api.Assertions.assertThat; + +import dev.openfeature.sdk.ImmutableContext; +import dev.openfeature.sdk.NoOpProvider; +import dev.openfeature.sdk.NoOpTransactionContextPropagator; +import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.OpenFeatureAPIFactory; +import dev.openfeature.sdk.ThreadLocalTransactionContextPropagator; +import dev.openfeature.sdk.providers.memory.Flag; +import dev.openfeature.sdk.providers.memory.InMemoryProvider; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +class IsolatedAPITest { + + /** + * Requirement 1.8.1 — factory creates new, distinct instances that + * conform to the API contract. + */ + @Test + @DisplayName("factory creates distinct API instances") + void factoryCreatesDistinctInstances() { + OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI(); + OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI(); + + assertThat(api1).isInstanceOf(OpenFeatureAPI.class).isNotSameAs(api2); + } + + /** + * Requirement 1.8.1 — providers are isolated between instances. + */ + @Test + @DisplayName("providers are isolated between instances") + void providerIsolation() { + OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI(); + OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI(); + + InMemoryProvider provider = new InMemoryProvider(Map.of()); + api1.setProvider(provider); + + assertThat(api1.getProvider()).isSameAs(provider); + assertThat(api2.getProvider()).isInstanceOf(NoOpProvider.class); + } + + /** + * Requirement 1.8.1 — hooks are isolated between instances. + */ + @Test + @DisplayName("hooks are isolated between instances") + void hookIsolation() { + OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI(); + OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI(); + + api1.addHooks(new NoOpHook()); + + assertThat(api1.getHooks()).hasSize(1); + assertThat(api2.getHooks()).isEmpty(); + } + + /** + * Requirement 1.8.1 — evaluation context is isolated between instances. + */ + @Test + @DisplayName("evaluation context is isolated between instances") + void evaluationContextIsolation() { + OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI(); + OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI(); + + api1.setEvaluationContext(new ImmutableContext("key-1")); + api2.setEvaluationContext(new ImmutableContext("key-2")); + + assertThat(api1.getEvaluationContext().getTargetingKey()).isEqualTo("key-1"); + assertThat(api2.getEvaluationContext().getTargetingKey()).isEqualTo("key-2"); + } + + /** + * Requirement 1.8.1 — event handlers are isolated between instances. + */ + @Test + @Timeout(value = 2, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) + @DisplayName("event handlers are isolated between instances") + void eventHandlerIsolation() throws Exception { + OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI(); + OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI(); + + CountDownLatch api1HandlerLatch = new CountDownLatch(1); + AtomicBoolean api2HandlerCalled = new AtomicBoolean(false); + + // Handlers are dispatched asynchronously; use a latch to await api1's handler. + api1.onProviderReady(details -> api1HandlerLatch.countDown()); + api2.onProviderReady(details -> api2HandlerCalled.set(true)); + + // setting a provider on api1 should only trigger api1's handler + api1.setProviderAndWait(new NoOpProvider()); + + assertThat(api1HandlerLatch.await(1, TimeUnit.SECONDS)).isTrue(); + assertThat(api2HandlerCalled.get()).isFalse(); + } + + /** + * Requirement 1.8.1 — transaction context propagators are isolated + * between instances. + */ + @Test + @DisplayName("transaction context propagator is isolated between instances") + void transactionContextPropagatorIsolation() { + OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI(); + OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI(); + + ThreadLocalTransactionContextPropagator propagator = new ThreadLocalTransactionContextPropagator(); + api1.setTransactionContextPropagator(propagator); + + assertThat(api1.getTransactionContextPropagator()).isSameAs(propagator); + assertThat(api2.getTransactionContextPropagator()).isInstanceOf(NoOpTransactionContextPropagator.class); + } + + /** + * Requirement 1.8.2 — an isolated instance conforms to the same API + * contract (provider, hooks, context, client creation, flag evaluation). + */ + @Test + @DisplayName("isolated instance conforms to API contract") + void isolatedInstanceConformsToAPIContract() throws Exception { + OpenFeatureAPI api = OpenFeatureAPIFactory.createAPI(); + + // provider management + InMemoryProvider provider = new InMemoryProvider(Map.of( + "flag1", + Flag.builder() + .variant("on", true) + .variant("off", false) + .defaultVariant("on") + .build())); + api.setProviderAndWait(provider); + assertThat(api.getProvider()).isSameAs(provider); + assertThat(api.getProviderMetadata()).isNotNull(); + + // hooks + NoOpHook hook = new NoOpHook(); + api.addHooks(hook); + assertThat(api.getHooks()).containsExactly(hook); + + // context + api.setEvaluationContext(new ImmutableContext("targeting-key")); + assertThat(api.getEvaluationContext().getTargetingKey()).isEqualTo("targeting-key"); + + // client creation and flag evaluation + var client = api.getClient("test-domain", "1.0"); + assertThat(client.getMetadata().getDomain()).isEqualTo("test-domain"); + assertThat(client.getBooleanValue("flag1", false)).isTrue(); + } + + /** + * Requirement 1.8.1 — clearHooks on one instance does not affect another. + */ + @Test + @DisplayName("clearHooks does not affect other instances") + void clearHooksDoesNotAffectOtherInstances() { + OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI(); + OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI(); + + NoOpHook hook = new NoOpHook(); + api1.addHooks(hook); + api2.addHooks(hook); + + api1.clearHooks(); + + assertThat(api1.getHooks()).isEmpty(); + assertThat(api2.getHooks()).hasSize(1); + } + + /** + * Requirement 1.8.2 — clients from different isolated instances use + * their own instance's provider. + */ + @Test + @DisplayName("clients use their own instance's provider") + void clientUsesItsOwnInstanceProvider() throws Exception { + OpenFeatureAPI api1 = OpenFeatureAPIFactory.createAPI(); + OpenFeatureAPI api2 = OpenFeatureAPIFactory.createAPI(); + + api1.setProviderAndWait(new InMemoryProvider(Map.of( + "flag1", + Flag.builder() + .variant("on", true) + .variant("off", false) + .defaultVariant("on") + .build()))); + + var client1 = api1.getClient(); + var client2 = api2.getClient(); + + assertThat(client1.getBooleanValue("flag1", false)).isTrue(); + // api2 has NoOpProvider, so it returns the default + assertThat(client2.getBooleanValue("flag1", false)).isFalse(); + } +} diff --git a/src/test/java/dev/openfeature/sdk/isolated/NoOpHook.java b/src/test/java/dev/openfeature/sdk/isolated/NoOpHook.java new file mode 100644 index 000000000..3aa0c76ed --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/isolated/NoOpHook.java @@ -0,0 +1,8 @@ +package dev.openfeature.sdk.isolated; + +import dev.openfeature.sdk.Hook; + +/** + * Minimal no-op hook for testing purposes. + */ +class NoOpHook implements Hook {}