diff --git a/mvnw b/mvnw old mode 100644 new mode 100755 diff --git a/pom.xml b/pom.xml index b4d8f35a8..7c0903da7 100644 --- a/pom.xml +++ b/pom.xml @@ -653,6 +653,26 @@ true all,-missing + + + + apiNote + a + API Note: + + + implSpec + a + Implementation Requirements: + + + implNote + a + Implementation Note: + + diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index c126c1451..beb2e1ce3 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -1,10 +1,12 @@ 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; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import lombok.extern.slf4j.Slf4j; /** @@ -29,21 +31,35 @@ 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 final AtomicReference attachment = new AtomicReference<>(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) { - // if we are trying to attach this provider to a different onEmit, something has gone wrong + void attach( + TriConsumer onEmit, + AutoCloseableReentrantReadWriteLock lock) { + Attachment newAttachment = new Attachment(onEmit, lock); + if (!this.attachment.compareAndSet(null, newAttachment)) { throw new IllegalStateException("Provider " + this.getMetadata().getName() + " is already attached."); - } else { - this.onEmit = onEmit; } } @@ -51,7 +67,7 @@ void attach(TriConsumer onEm * "Detach" this EventProvider from an SDK, stopping propagation of all events. */ void detach() { - this.onEmit = null; + this.attachment.set(null); } /** @@ -80,9 +96,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.get(); - if (localEventProviderListener == null && localOnEmit == null) { + if (localEventProviderListener == null && localAttachment == null) { return Awaitable.FINISHED; } @@ -91,12 +107,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 04ad504c3..209cfe4e8 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -9,28 +9,68 @@ 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; import lombok.extern.slf4j.Slf4j; /** - * A global singleton which holds base configuration for the OpenFeature - * library. - * Configuration here will be shared across all {@link Client}s. + * Holds base configuration for the OpenFeature library. + * + *

Most applications should use the global singleton via {@link #getInstance()}; configuration + * there is shared across all {@link Client}s. For dependency-injection frameworks, testing, or + * multi-tenant scenarios that need fully independent state (providers, hooks, evaluation context, + * event handlers, transaction context propagators), create isolated instances via + * {@link #createIsolated()}. + * + *

Note: Isolated API instances (per spec section 1.8) are experimental and + * subject to change. + * + * @see + * Spec §1.8 — Isolated API Instances */ @Slf4j @SuppressWarnings("PMD.UnusedLocalVariable") public class OpenFeatureAPI implements EventBus { - // 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 GLOBAL_PROVIDER_REGISTRY = + new ConcurrentHashMap<>(); + + // package-private multi-read/single-write lock (instance-level for isolation) + final AutoCloseableReentrantReadWriteLock lock; private final ConcurrentLinkedQueue apiHooks; private ProviderRepository providerRepository; private EventSupport eventSupport; private final AtomicReference evaluationContext = new AtomicReference<>(); private TransactionContextPropagator transactionContextPropagator; - protected OpenFeatureAPI() { + /** + * Creates and returns a new, independent {@link OpenFeatureAPI} instance with fully isolated + * state (providers, hooks, evaluation context, event handlers, transaction context + * propagators). + * + * @apiNote This API is experimental and subject to change. + * @see + * Spec §1.8 — Isolated API Instances + */ + public static OpenFeatureAPI createIsolated() { + return new OpenFeatureAPI(); + } + + // Package-private: not part of the public API; use createIsolated(). + 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 +291,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); } } @@ -332,6 +372,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 diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 3a0d325df..2e1d75d2c 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -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; } @@ -236,6 +238,8 @@ private void initializeProvider( private void shutDownOld(FeatureProviderStateManager oldManager, Consumer 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()); } @@ -327,7 +331,11 @@ List prepareShutdown() { * @param managersToShutdown the managers to shut down (from prepareShutdown) */ void completeShutdown(List 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)) { diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index d04fa88d1..a90d30bbd 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -1,12 +1,16 @@ package dev.openfeature.sdk; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; 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; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -36,7 +40,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,17 +77,39 @@ 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 - @DisplayName("should not throw if second same onEmit attached") - void doesNotThrowWhenOnEmitSame() { - TriConsumer onEmit1 = mockOnEmit(); - TriConsumer onEmit2 = onEmit1; - eventProvider.attach(onEmit1); - eventProvider.attach(onEmit2); // should not throw, same instance. noop + @Timeout(value = 2, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) + @DisplayName("emit should acquire read lock when attached") + void emitAcquiresReadLockWhenAttached() throws Exception { + AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock(); + CountDownLatch lockAcquired = new CountDownLatch(1); + + TriConsumer onEmit = (ep, event, details) -> { + // When the onEmit callback runs, the read lock must already be held + assertThat(lock.getReadLockCount()).isGreaterThan(0); + lockAcquired.countDown(); + }; + + eventProvider.attach(onEmit, lock); + eventProvider.emit( + ProviderEvent.PROVIDER_READY, ProviderEventDetails.builder().build()); + + assertThat(lockAcquired.await(1, TimeUnit.SECONDS)).isTrue(); + } + + @Test + @Timeout(value = 2, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) + @DisplayName("emit should not acquire lock when not attached") + void emitDoesNotAcquireLockWhenNotAttached() { + // emit without attaching — should return immediately without error + Awaitable result = eventProvider.emit( + ProviderEvent.PROVIDER_READY, ProviderEventDetails.builder().build()); + assertThat(result).isSameAs(Awaitable.FINISHED); } @Test @@ -132,8 +158,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/GlobalProviderRegistryTest.java b/src/test/java/dev/openfeature/sdk/GlobalProviderRegistryTest.java new file mode 100644 index 000000000..a98e9ce96 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/GlobalProviderRegistryTest.java @@ -0,0 +1,106 @@ +package dev.openfeature.sdk; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.Mockito.never; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.simplify4u.slf4jmock.LoggerMock; +import org.slf4j.Logger; + +class GlobalProviderRegistryTest { + + /** + * Re-registering a provider with the same API instance should not produce a warning. + * This exercises the {@code existing == this} path in registerGlobalProvider. + */ + @Test + @DisplayName("no warning when same API instance re-registers the same provider") + void noWarningWhenSameInstanceReRegisters() { + Logger mockLogger = Mockito.mock(Logger.class); + LoggerMock.setMock(OpenFeatureAPI.class, mockLogger); + try { + OpenFeatureAPI api = new OpenFeatureAPI(); + NoOpProvider provider = new NoOpProvider(); + + api.registerGlobalProvider(provider); + api.registerGlobalProvider(provider); // same instance, second call + + Mockito.verify(mockLogger, never()).warn(Mockito.anyString()); + } finally { + LoggerMock.setMock(OpenFeatureAPI.class, null); + } + } + + /** + * After deregistering a provider, binding it to a different API instance + * should not produce a warning — proving the entry was removed. + */ + @Test + @DisplayName("deregister removes provider from global registry") + void deregisterRemovesProviderFromRegistry() { + Logger mockLogger = Mockito.mock(Logger.class); + LoggerMock.setMock(OpenFeatureAPI.class, mockLogger); + try { + OpenFeatureAPI api1 = new OpenFeatureAPI(); + OpenFeatureAPI api2 = new OpenFeatureAPI(); + NoOpProvider provider = new NoOpProvider(); + + api1.registerGlobalProvider(provider); + api1.deregisterGlobalProvider(provider); + + // Should not warn because the provider was deregistered + api2.registerGlobalProvider(provider); + + Mockito.verify(mockLogger, never()).warn(Mockito.anyString()); + } finally { + LoggerMock.setMock(OpenFeatureAPI.class, null); + } + } + + /** + * Deregister is a no-op if the calling instance is not the current owner. + * The original owner's registration should remain intact. + */ + @Test + @DisplayName("deregister is a no-op when called by non-owner instance") + void deregisterIsNoOpForNonOwner() { + Logger mockLogger = Mockito.mock(Logger.class); + LoggerMock.setMock(OpenFeatureAPI.class, mockLogger); + try { + OpenFeatureAPI api1 = new OpenFeatureAPI(); + OpenFeatureAPI api2 = new OpenFeatureAPI(); + NoOpProvider provider = new NoOpProvider(); + + api1.registerGlobalProvider(provider); + + // api2 is not the owner — this should be a no-op + api2.deregisterGlobalProvider(provider); + + // api2 re-registering should still warn, because api1 still owns it + api2.registerGlobalProvider(provider); + Mockito.verify(mockLogger).warn(contains("1.8.4")); + } finally { + LoggerMock.setMock(OpenFeatureAPI.class, null); + } + } + + /** + * Calling shutdown() twice on an API instance should be safe (idempotent). + * The second call returns early because prepareShutdown returns null. + */ + @Test + @DisplayName("double shutdown on API instance is safe") + void doubleShutdownIsSafe() { + OpenFeatureAPI api = new OpenFeatureAPI(); + api.setProvider(new NoOpProvider()); + + assertThatCode(() -> { + api.shutdown(); + api.shutdown(); + }) + .doesNotThrowAnyException(); + } +} 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..84e21f9df --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/isolated/IsolatedAPISingeltonTest.java @@ -0,0 +1,76 @@ +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.ThreadLocalTransactionContextPropagator; +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 = OpenFeatureAPI.createIsolated(); + 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(); + } + + /** + * Requirement 1.8.1 — mutating the singleton does not affect a + * previously created isolated instance. + */ + @Test + @DisplayName("singleton does not interfere with isolated instance") + void singletonDoesNotInterfereWithIsolatedInstance() { + OpenFeatureAPI isolated = OpenFeatureAPI.createIsolated(); + + // record isolated baseline + FeatureProvider isolatedProvider = isolated.getProvider(); + + // mutate only the singleton + singleton.setProvider(new InMemoryProvider(Map.of())); + singleton.addHooks(new NoOpHook()); + singleton.setEvaluationContext(new ImmutableContext("singleton-key")); + singleton.setTransactionContextPropagator(new ThreadLocalTransactionContextPropagator()); + + // isolated instance remains at baseline + assertThat(isolated.getProvider()).isSameAs(isolatedProvider); + assertThat(isolated.getHooks()).isEmpty(); + assertThat(isolated.getEvaluationContext()).isNull(); + assertThat(isolated.getTransactionContextPropagator()).isInstanceOf(NoOpTransactionContextPropagator.class); + } +} 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..f9d7ef2e7 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/isolated/IsolatedAPITest.java @@ -0,0 +1,235 @@ +package dev.openfeature.sdk.isolated; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.contains; + +import dev.openfeature.sdk.ImmutableContext; +import dev.openfeature.sdk.NoOpProvider; +import dev.openfeature.sdk.NoOpTransactionContextPropagator; +import dev.openfeature.sdk.OpenFeatureAPI; +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; +import org.mockito.Mockito; +import org.simplify4u.slf4jmock.LoggerMock; +import org.slf4j.Logger; + +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 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + 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 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + 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 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + 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 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + 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 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + 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(); + + // Short delay to allow any (incorrect) async dispatch to api2 to land + // before we assert the negative. Without this, a bug in isolation could + // slip past if api2's handler fires just after the assertion. + Thread.sleep(200); + 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 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + 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 = OpenFeatureAPI.createIsolated(); + + // 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 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + 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 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + 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(); + } + + /** + * Requirement 1.8.4 — a warning is logged when the same provider instance + * is registered with more than one API instance simultaneously. + */ + @Test + @DisplayName("warn when same provider bound to multiple API instances (req 1.8.4)") + void warnWhenProviderBoundToMultipleInstances() { + Logger mockLogger = Mockito.mock(Logger.class); + LoggerMock.setMock(OpenFeatureAPI.class, mockLogger); + try { + OpenFeatureAPI api1 = OpenFeatureAPI.createIsolated(); + OpenFeatureAPI api2 = OpenFeatureAPI.createIsolated(); + + NoOpProvider provider = new NoOpProvider(); + api1.setProvider(provider); + api2.setProvider(provider); + + Mockito.verify(mockLogger).warn(contains("1.8.4")); + } finally { + LoggerMock.setMock(OpenFeatureAPI.class, null); + } + } +} 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 {}