diff --git a/src/main/java/dev/openfeature/sdk/FeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProvider.java index 22819ef10..902c0d7d1 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProvider.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import javax.annotation.Nullable; /** * The interface implemented by upstream flag providers to resolve flags for @@ -41,6 +42,38 @@ default void initialize(EvaluationContext evaluationContext) throws Exception { // Intentionally left blank } + /** + * This method is called before a provider is used to evaluate flags, with the + * bound domain supplied when the provider is registered to a named client. + * + *

+ * The default provider is initialized with a {@code null} domain. Providers that + * maintain per-domain state (for example a persistent cache) should override this + * method and declare themselves {@linkplain #isDomainScoped() domain-scoped}. + *

+ * + * @param evaluationContext the global evaluation context + * @param domain the bound domain, or {@code null} for the default provider + */ + default void initialize(EvaluationContext evaluationContext, @Nullable String domain) throws Exception { + initialize(evaluationContext); + } + + /** + * Returns whether this provider maintains state specific to a single domain that + * cannot be shared across domains. + * + *

+ * Domain-scoped providers may only be bound to one domain within a single API + * instance. + *

+ * + * @return {@code true} if this provider is domain-scoped + */ + default boolean isDomainScoped() { + return false; + } + /** * This method is called when a new provider is about to be used to evaluate * flags, or the SDK is shut down. diff --git a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java index 5fd70221b..4cfda2c45 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java @@ -3,6 +3,7 @@ import dev.openfeature.sdk.exceptions.OpenFeatureError; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.Nullable; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -19,11 +20,15 @@ public FeatureProviderStateManager(FeatureProvider delegate) { } public void initialize(EvaluationContext evaluationContext) throws Exception { + initialize(evaluationContext, null); + } + + public void initialize(EvaluationContext evaluationContext, @Nullable String domain) throws Exception { if (isInitialized.getAndSet(true)) { return; } try { - delegate.initialize(evaluationContext); + delegate.initialize(evaluationContext, domain); setState(ProviderState.READY); } catch (OpenFeatureError openFeatureError) { if (ErrorCode.PROVIDER_FATAL.equals(openFeatureError.getErrorCode())) { diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 7fcb8f434..c94100008 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -17,6 +17,7 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; +import javax.annotation.Nullable; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -154,7 +155,7 @@ public void setProvider( } private void prepareAndInitializeProvider( - String domain, + @Nullable String domain, FeatureProvider newProvider, Consumer afterSet, Consumer afterInit, @@ -169,6 +170,7 @@ private void prepareAndInitializeProvider( throw new IllegalStateException("Provider cannot be set while repository is shutting down"); } FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider); + validateDomainScopedBinding(domain, newProvider); if (existing == null) { openFeatureAPI.registerGlobalProvider(newProvider); newStateManager = new FeatureProviderStateManager(newProvider); @@ -185,29 +187,74 @@ private void prepareAndInitializeProvider( } if (waitForInit) { - initializeProvider(newStateManager, afterInit, afterShutdown, afterError, oldStateManager); + initializeProvider(domain, newStateManager, afterInit, afterShutdown, afterError, oldStateManager); } else { taskExecutor.submit(() -> { // initialization happens in a different thread if we're not waiting for it - initializeProvider(newStateManager, afterInit, afterShutdown, afterError, oldStateManager); + initializeProvider(domain, newStateManager, afterInit, afterShutdown, afterError, oldStateManager); }); } } + private void validateDomainScopedBinding(@Nullable String domain, FeatureProvider newProvider) { + if (!newProvider.isDomainScoped()) { + return; + } + + boolean currentlyDefault = isDefaultProviderInstance(newProvider); + List boundNamedDomains = getBoundDomainsForProviderInstance(newProvider); + + if (!currentlyDefault && boundNamedDomains.isEmpty()) { + return; + } + + if (domain == null) { + if (!currentlyDefault) { + throw new IllegalArgumentException("Domain-scoped provider cannot be bound to more than one domain"); + } + return; + } + if (boundNamedDomains.contains(domain)) { + return; + } + if (!boundNamedDomains.isEmpty() || currentlyDefault) { + throw new IllegalArgumentException("Domain-scoped provider cannot be bound to more than one domain"); + } + } + + private boolean isDefaultProviderInstance(FeatureProvider provider) { + return defaultStateManger.get().getProvider() == provider; + } + + private List getBoundDomainsForProviderInstance(FeatureProvider provider) { + return stateManagers.entrySet().stream() + .filter(entry -> entry.getValue().getProvider() == provider) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); + } + private FeatureProviderStateManager getExistingStateManagerForProvider(FeatureProvider provider) { for (FeatureProviderStateManager stateManager : stateManagers.values()) { - if (stateManager.hasSameProvider(provider)) { + if (matchesProvider(stateManager.getProvider(), provider)) { return stateManager; } } FeatureProviderStateManager defaultFeatureProviderStateManager = defaultStateManger.get(); - if (defaultFeatureProviderStateManager.hasSameProvider(provider)) { + if (matchesProvider(defaultFeatureProviderStateManager.getProvider(), provider)) { return defaultFeatureProviderStateManager; } return null; } + private boolean matchesProvider(FeatureProvider registered, FeatureProvider candidate) { + if (candidate.isDomainScoped()) { + return registered == candidate; + } + return registered.equals(candidate); + } + private void initializeProvider( + @Nullable String domain, FeatureProviderStateManager newManager, Consumer afterInit, Consumer afterShutdown, @@ -215,7 +262,7 @@ private void initializeProvider( FeatureProviderStateManager oldManager) { try { if (ProviderState.NOT_READY.equals(newManager.getState())) { - newManager.initialize(openFeatureAPI.getEvaluationContext()); + newManager.initialize(openFeatureAPI.getEvaluationContext(), domain); afterInit.accept(newManager.getProvider()); } shutDownOld(oldManager, afterShutdown); diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java index cc6fb8db2..f1389b67e 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java @@ -19,6 +19,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import javax.annotation.Nullable; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -84,6 +85,19 @@ protected static Map buildProviders(List providersMetadata = new HashMap<>(); @@ -98,7 +112,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { Collection> tasks = new ArrayList<>(providers.size()); for (FeatureProvider provider : providers.values()) { tasks.add(() -> { - provider.initialize(evaluationContext); + provider.initialize(evaluationContext, domain); return null; }); Metadata providerMetadata = provider.getMetadata(); diff --git a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java new file mode 100644 index 000000000..1115ff4e9 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java @@ -0,0 +1,250 @@ +package dev.openfeature.sdk; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.awaitility.Awaitility.await; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.time.Duration; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +class DomainScopedProviderSpecTest { + + private static final String DOMAIN_A = "domain-a"; + private static final String DOMAIN_B = "domain-b"; + private OpenFeatureAPI api; + + @BeforeEach + void setupTest() { + api = new OpenFeatureAPI(); + api.setProvider(new NoOpProvider()); + } + + @AfterEach + void tearDown() { + api.shutdown(); + } + + @Specification( + number = "1.1.8.1", + text = "The `provider mutator` MUST NOT bind a `domain-scoped` provider instance to more than one " + + "`domain`, rejecting any attempt to bind an already-bound instance to an additional `domain`.") + @Test + @DisplayName("rejects binding a domain-scoped provider to a second named domain") + void rejectsBindingDomainScopedProviderToSecondNamedDomain() { + DomainScopedTestProvider provider = new DomainScopedTestProvider(); + + api.setProvider(DOMAIN_A, provider); + + assertThatThrownBy(() -> api.setProvider(DOMAIN_B, provider)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Domain-scoped provider cannot be bound to more than one domain"); + } + + @Specification( + number = "1.1.8.1", + text = "The `provider mutator` MUST NOT bind a `domain-scoped` provider instance to more than one " + + "`domain`, rejecting any attempt to bind an already-bound instance to an additional `domain`.") + @Test + @DisplayName("rejects binding a domain-scoped named provider as the default provider") + void rejectsBindingDomainScopedNamedProviderAsDefault() { + DomainScopedTestProvider provider = new DomainScopedTestProvider(); + + api.setProvider(DOMAIN_A, provider); + + assertThatThrownBy(() -> api.setProvider(provider)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Domain-scoped provider cannot be bound to more than one domain"); + } + + @Specification( + number = "1.1.8.1", + text = "The `provider mutator` MUST NOT bind a `domain-scoped` provider instance to more than one " + + "`domain`, rejecting any attempt to bind an already-bound instance to an additional `domain`.") + @Test + @DisplayName("rejects binding a domain-scoped default provider to a named domain") + void rejectsBindingDomainScopedDefaultProviderToNamedDomain() { + DomainScopedTestProvider provider = new DomainScopedTestProvider(); + + api.setProvider(provider); + + assertThatThrownBy(() -> api.setProvider(DOMAIN_A, provider)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Domain-scoped provider cannot be bound to more than one domain"); + } + + @Test + @DisplayName("allows rebinding a domain-scoped provider to the same named domain") + void allowsRebindingDomainScopedProviderToSameNamedDomain() throws Exception { + DomainScopedTestProvider provider = new DomainScopedTestProvider(); + FeatureProvider replacement = mock(FeatureProvider.class); + doReturn(ProviderState.NOT_READY).when(replacement).getState(); + doReturn(true).when(replacement).isDomainScoped(); + + api.setProvider(DOMAIN_A, provider); + + assertThatCode(() -> api.setProvider(DOMAIN_A, replacement)).doesNotThrowAnyException(); + verify(replacement, timeout(1000)).initialize(any(), eq(DOMAIN_A)); + } + + @Test + @DisplayName("allows rebinding the same domain-scoped provider instance to the same named domain") + void allowsRebindingSameDomainScopedInstanceToSameNamedDomain() throws Exception { + DomainScopedTestProvider provider = new DomainScopedTestProvider(); + + api.setProviderAndWait(DOMAIN_A, provider); + api.setProviderAndWait(DOMAIN_A, provider); + + assertThat(provider.initCount()).isOne(); + assertThat(provider.initDomain.get()).isEqualTo(DOMAIN_A); + } + + @Test + @DisplayName("allows rebinding the same domain-scoped provider instance as the default provider") + void allowsRebindingSameDomainScopedInstanceAsDefault() throws Exception { + DomainScopedTestProvider provider = new DomainScopedTestProvider(); + + api.setProviderAndWait(provider); + api.setProviderAndWait(provider); + + assertThat(provider.initCount()).isOne(); + assertThat(provider.initDomain.get()).isNull(); + } + + @Test + @DisplayName("allows equal-but-distinct domain-scoped instances on separate domains") + void allowsEqualDistinctDomainScopedInstancesOnSeparateDomains() throws Exception { + EqualDomainScopedProvider providerA = new EqualDomainScopedProvider("shared-key"); + EqualDomainScopedProvider providerB = new EqualDomainScopedProvider("shared-key"); + + assertThat(providerA).isEqualTo(providerB); + assertThat(providerA).isNotSameAs(providerB); + + api.setProviderAndWait(DOMAIN_A, providerA); + + assertThatCode(() -> api.setProviderAndWait(DOMAIN_B, providerB)).doesNotThrowAnyException(); + assertThat(api.getProvider(DOMAIN_A)).isSameAs(providerA); + assertThat(api.getProvider(DOMAIN_B)).isSameAs(providerB); + } + + @Test + @DisplayName("allows binding a non-domain-scoped provider to multiple domains") + void allowsBindingNonDomainScopedProviderToMultipleDomains() throws Exception { + FeatureProvider provider = mock(FeatureProvider.class); + doReturn(ProviderState.NOT_READY).when(provider).getState(); + Metadata metadata = mock(Metadata.class); + doReturn("shared-provider").when(metadata).getName(); + doReturn(metadata).when(provider).getMetadata(); + + api.setProviderAndWait(DOMAIN_A, provider); + api.setProviderAndWait(DOMAIN_B, provider); + + assertThat(api.getProvider(DOMAIN_A)).isSameAs(provider); + assertThat(api.getProvider(DOMAIN_B)).isSameAs(provider); + verify(provider, times(1)).initialize(any(), eq(DOMAIN_A)); + } + + @Specification( + number = "2.4.3", + text = + "The `provider` MAY declare that it is `domain-scoped`, indicating that it maintains state " + + "specific to a single `domain`, such as a persistent cache, that cannot be shared across `domains`.") + @Test + @DisplayName("domain-scoped provider receives bound domain at initialization") + void domainScopedProviderReceivesBoundDomainAtInitialization() { + DomainScopedTestProvider provider = new DomainScopedTestProvider(); + + api.setProvider(DOMAIN_A, provider); + + await().atMost(Duration.ofSeconds(5)) + .untilAsserted(() -> assertThat(provider.initDomain.get()).isEqualTo(DOMAIN_A)); + } + + private static class DomainScopedTestProvider extends EventProvider { + + private final AtomicReference initDomain = new AtomicReference<>(); + private final AtomicInteger initializeCount = new AtomicInteger(); + + @Override + public Metadata getMetadata() { + return () -> "domain-scoped-test"; + } + + @Override + public boolean isDomainScoped() { + return true; + } + + @Override + public void initialize(EvaluationContext evaluationContext, String domain) { + initializeCount.incrementAndGet(); + initDomain.set(domain); + } + + @Override + public ProviderEvaluation getBooleanEvaluation( + String key, Boolean defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getIntegerEvaluation( + String key, Integer defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + int initCount() { + return initializeCount.get(); + } + } + + private static final class EqualDomainScopedProvider extends DomainScopedTestProvider { + + private final String key; + + EqualDomainScopedProvider(String key) { + this.key = key; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof EqualDomainScopedProvider)) { + return false; + } + EqualDomainScopedProvider other = (EqualDomainScopedProvider) obj; + return key.equals(other.key); + } + + @Override + public int hashCode() { + return key.hashCode(); + } + } +} diff --git a/src/test/java/dev/openfeature/sdk/FeatureProviderStateManagerTest.java b/src/test/java/dev/openfeature/sdk/FeatureProviderStateManagerTest.java index ff3f3a3f8..6e0008cd5 100644 --- a/src/test/java/dev/openfeature/sdk/FeatureProviderStateManagerTest.java +++ b/src/test/java/dev/openfeature/sdk/FeatureProviderStateManagerTest.java @@ -6,6 +6,7 @@ import dev.openfeature.sdk.exceptions.FatalError; import dev.openfeature.sdk.exceptions.GeneralError; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; @@ -22,6 +23,13 @@ public void setUp() { wrapper = new FeatureProviderStateManager(testDelegate); } + @SneakyThrows + @Test + void shouldPassDomainToDelegateOnInit() { + wrapper.initialize(null, "billing"); + assertThat(testDelegate.initDomain.get()).isEqualTo("billing"); + } + @SneakyThrows @Test void shouldOnlyCallInitOnce() { @@ -156,6 +164,7 @@ void shouldSetTheStateToFatalWhenAFatalErrorEventIsEmitted() { static class TestDelegate extends EventProvider { private final AtomicInteger initCalled = new AtomicInteger(); private final AtomicInteger shutdownCalled = new AtomicInteger(); + private final AtomicReference initDomain = new AtomicReference<>(); private @Nullable Exception throwOnInit; @Override @@ -190,6 +199,12 @@ public ProviderEvaluation getObjectEvaluation(String key, Value defaultVa return null; } + @Override + public void initialize(EvaluationContext evaluationContext, String domain) throws Exception { + initDomain.set(domain); + initialize(evaluationContext); + } + @Override public void initialize(EvaluationContext evaluationContext) throws Exception { initCalled.incrementAndGet(); diff --git a/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java b/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java index 4bcd73127..1a806a816 100644 --- a/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java @@ -1,7 +1,9 @@ package dev.openfeature.sdk; import static org.assertj.core.api.Assertions.assertThatCode; -import static org.mockito.Mockito.any; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -41,7 +43,22 @@ void mustCallInitializeFunctionOfTheNewlyRegisteredProviderBeforeUsingItForFlagE api.setProvider(featureProvider); - verify(featureProvider, timeout(1000)).initialize(any()); + verify(featureProvider, timeout(1000)).initialize(any(), isNull()); + } + + @Specification( + number = "2.4.1", + text = "The `provider` MAY define an initialization function which accepts the global " + + "`evaluation context` and an optional bound `domain`.") + @Test + @DisplayName("must pass null domain when initializing the default provider") + void mustPassNullDomainWhenInitializingTheDefaultProvider() throws Exception { + FeatureProvider featureProvider = mock(FeatureProvider.class); + doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); + + api.setProvider(featureProvider); + + verify(featureProvider, timeout(1000)).initialize(any(), isNull()); } @Specification( @@ -55,11 +72,11 @@ void mustCallInitializeFunctionOfTheNewlyRegisteredProviderBeforeUsingItForFlagE void shouldCatchExceptionThrownByTheProviderOnInitialization() throws Exception { FeatureProvider featureProvider = mock(FeatureProvider.class); doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); - doThrow(TestException.class).when(featureProvider).initialize(any()); + doThrow(TestException.class).when(featureProvider).initialize(any(), isNull()); assertThatCode(() -> api.setProvider(featureProvider)).doesNotThrowAnyException(); - verify(featureProvider, timeout(1000)).initialize(any()); + verify(featureProvider, timeout(1000)).initialize(any(), isNull()); } } @@ -80,7 +97,22 @@ void mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItFor api.setProvider(DOMAIN_NAME, featureProvider); - verify(featureProvider, timeout(1000)).initialize(any()); + verify(featureProvider, timeout(1000)).initialize(any(), eq(DOMAIN_NAME)); + } + + @Specification( + number = "2.4.1", + text = "The `provider` MAY define an initialization function which accepts the global " + + "`evaluation context` and an optional bound `domain`.") + @Test + @DisplayName("must pass bound domain when initializing a named provider") + void mustPassBoundDomainWhenInitializingANamedProvider() throws Exception { + FeatureProvider featureProvider = mock(FeatureProvider.class); + doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); + + api.setProvider(DOMAIN_NAME, featureProvider); + + verify(featureProvider, timeout(1000)).initialize(any(), eq(DOMAIN_NAME)); } @Specification( @@ -94,11 +126,11 @@ void mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItFor void shouldCatchExceptionThrownByTheNamedClientProviderOnInitialization() throws Exception { FeatureProvider featureProvider = mock(FeatureProvider.class); doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); - doThrow(TestException.class).when(featureProvider).initialize(any()); + doThrow(TestException.class).when(featureProvider).initialize(any(), eq(DOMAIN_NAME)); assertThatCode(() -> api.setProvider(DOMAIN_NAME, featureProvider)).doesNotThrowAnyException(); - verify(featureProvider, timeout(1000)).initialize(any()); + verify(featureProvider, timeout(1000)).initialize(any(), eq(DOMAIN_NAME)); } } } diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java index b1f5b6ed8..4b15458fa 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java @@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -117,7 +118,7 @@ void featureProviderTrackIsCalled() throws Exception { api.getClient().track("track-event", new ImmutableContext(), new MutableTrackingEventDetails(22.2f)); - verify(featureProvider).initialize(any()); + verify(featureProvider).initialize(any(), isNull()); verify(featureProvider, times(2)).getMetadata(); verify(featureProvider).track(any(), any(), any()); } diff --git a/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java b/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java new file mode 100644 index 000000000..35b4428d0 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java @@ -0,0 +1,241 @@ +package dev.openfeature.sdk; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +import java.time.Duration; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +class ProviderInitializeBackwardCompatibilityTest { + + private static final String DOMAIN = "billing"; + + @Nested + @DisplayName("FeatureProvider default method delegation") + class DefaultMethodDelegation { + + @Test + @DisplayName("default isDomainScoped returns false for non-domain-scoped providers") + void defaultIsDomainScopedReturnsFalse() { + assertThat(new LegacySingleArgInitProvider().isDomainScoped()).isFalse(); + } + + @Test + @DisplayName("two-arg default delegates to a single-arg override") + void twoArgDefaultDelegatesToSingleArgOverride() throws Exception { + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); + + provider.initialize(new ImmutableContext(), DOMAIN); + + assertThat(provider.singleArgInitCount()).isOne(); + } + + @Test + @DisplayName("two-arg override is used without invoking single-arg") + void twoArgOverrideDoesNotInvokeSingleArg() throws Exception { + TwoArgInitProvider provider = new TwoArgInitProvider(); + + provider.initialize(new ImmutableContext(), DOMAIN); + + assertThat(provider.initDomain()).isEqualTo(DOMAIN); + assertThat(provider.singleArgInitCount()).isZero(); + } + + @Test + @DisplayName("two-arg-only override receives null domain for default binding") + void twoArgOnlyOverrideReceivesNullDomain() throws Exception { + TwoArgInitProvider provider = new TwoArgInitProvider(); + + provider.initialize(new ImmutableContext(), null); + + assertThat(provider.initDomain()).isNull(); + assertThat(provider.singleArgInitCount()).isZero(); + } + + @Test + @DisplayName("single-arg override is only invoked once per initialization") + void singleArgOverrideIsOnlyInvokedOnce() throws Exception { + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); + + provider.initialize(new ImmutableContext(), DOMAIN); + provider.initialize(new ImmutableContext(), DOMAIN); + + assertThat(provider.singleArgInitCount()).isEqualTo(2); + } + } + + @Nested + @DisplayName("SDK initialization path") + class SdkIntegration { + + private OpenFeatureAPI api; + + @BeforeEach + void setupTest() { + api = new OpenFeatureAPI(); + api.setProvider(new NoOpProvider()); + } + + @Test + @DisplayName("legacy single-arg provider is initialized when registered to a named domain") + void legacySingleArgProviderInitializedForNamedDomain() { + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); + + api.setProvider(DOMAIN, provider); + + await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> assertThat(provider.singleArgInitCount()) + .isOne()); + } + + @Test + @DisplayName("legacy single-arg provider is initialized when registered as the default provider") + void legacySingleArgProviderInitializedAsDefault() { + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); + + api.setProvider(provider); + + await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> assertThat(provider.singleArgInitCount()) + .isOne()); + } + + @Test + @DisplayName("two-arg-only provider receives the bound domain from the SDK") + void twoArgOnlyProviderReceivesBoundDomainFromSdk() { + TwoArgInitProvider provider = new TwoArgInitProvider(); + + api.setProvider(DOMAIN, provider); + + await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { + assertThat(provider.initDomain()).isEqualTo(DOMAIN); + assertThat(provider.singleArgInitCount()).isZero(); + }); + } + + @Test + @DisplayName("two-arg-only default provider receives null domain from the SDK") + void twoArgOnlyDefaultProviderReceivesNullDomainFromSdk() { + TwoArgInitProvider provider = new TwoArgInitProvider(); + + api.setProvider(provider); + + await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { + assertThat(provider.initDomain()).isNull(); + assertThat(provider.singleArgInitCount()).isZero(); + }); + } + } + + @Nested + @DisplayName("FeatureProviderStateManager") + class StateManager { + + @Test + @DisplayName("delegates two-arg initialize to a legacy single-arg provider") + void delegatesTwoArgInitializeToLegacySingleArgProvider() throws Exception { + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); + FeatureProviderStateManager stateManager = new FeatureProviderStateManager(provider); + + stateManager.initialize(new ImmutableContext(), DOMAIN); + + assertThat(provider.singleArgInitCount()).isOne(); + } + + @Test + @DisplayName("only invokes two-arg initialize once for a legacy single-arg provider") + void onlyInvokesLegacySingleArgProviderOncePerStateManagerInit() throws Exception { + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); + FeatureProviderStateManager stateManager = new FeatureProviderStateManager(provider); + + stateManager.initialize(new ImmutableContext(), DOMAIN); + stateManager.initialize(new ImmutableContext(), DOMAIN); + + assertThat(provider.singleArgInitCount()).isOne(); + } + } + + /** + * Legacy provider that only overrides single-arg {@link FeatureProvider#initialize(EvaluationContext)}. + */ + private static final class LegacySingleArgInitProvider extends StubProvider { + + private final AtomicInteger singleArgInitCount = new AtomicInteger(); + + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + + int singleArgInitCount() { + return singleArgInitCount.get(); + } + } + + /** + * Domain-aware provider that only overrides two-arg + * {@link FeatureProvider#initialize(EvaluationContext, String)}. Single-arg is overridden solely to detect + * accidental delegation. + */ + private static final class TwoArgInitProvider extends StubProvider { + + private final AtomicInteger singleArgInitCount = new AtomicInteger(); + private final AtomicReference initDomain = new AtomicReference<>(); + + @Override + public void initialize(EvaluationContext evaluationContext, String domain) { + initDomain.set(domain); + } + + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + + int singleArgInitCount() { + return singleArgInitCount.get(); + } + + String initDomain() { + return initDomain.get(); + } + } + + private abstract static class StubProvider extends EventProvider { + + @Override + public Metadata getMetadata() { + return () -> "initialize-backward-compat-test"; + } + + @Override + public ProviderEvaluation getBooleanEvaluation( + String key, Boolean defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getIntegerEvaluation( + String key, Integer defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + } +} diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index c9d69ad73..a4ca798f5 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -8,6 +8,7 @@ import static org.awaitility.Awaitility.await; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.*; import dev.openfeature.sdk.exceptions.OpenFeatureError; @@ -64,7 +65,7 @@ void shouldHaveNoOpProviderSetAsDefaultOnInitialization() { @DisplayName("should immediately return when calling the provider mutator") void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { FeatureProvider featureProvider = createMockedProvider(); - doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(new ImmutableContext()); + doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(any(), isNull()); await().alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) @@ -77,11 +78,11 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { mockAfterShutdown(), mockAfterError(), false); - verify(featureProvider, timeout(TIMEOUT)).initialize(any()); + verify(featureProvider, timeout(TIMEOUT)).initialize(any(), isNull()); return true; }); - verify(featureProvider, timeout(TIMEOUT)).initialize(any()); + verify(featureProvider, timeout(TIMEOUT)).initialize(any(), isNull()); } } @@ -121,7 +122,7 @@ void shouldRejectNullAsDefaultProvider() { @DisplayName("should immediately return when calling the domain provider mutator") void shouldImmediatelyReturnWhenCallingTheDomainProviderMutator() throws Exception { FeatureProvider featureProvider = createMockedProvider(); - doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(any()); + doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(any(), eq("a domain")); await().alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) @@ -135,7 +136,7 @@ void shouldImmediatelyReturnWhenCallingTheDomainProviderMutator() throws Excepti mockAfterShutdown(), mockAfterError(), false); - verify(featureProvider, timeout(TIMEOUT)).initialize(any()); + verify(featureProvider, timeout(TIMEOUT)).initialize(any(), eq("a domain")); return true; }); } @@ -152,7 +153,7 @@ class DefaultProvider { @DisplayName("should immediately return when calling the provider mutator") void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { FeatureProvider newProvider = createMockedProvider(); - doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any()); + doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any(), isNull()); await().alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) @@ -165,11 +166,11 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { mockAfterShutdown(), mockAfterError(), false); - verify(newProvider, timeout(TIMEOUT)).initialize(any()); + verify(newProvider, timeout(TIMEOUT)).initialize(any(), isNull()); return true; }); - verify(newProvider, timeout(TIMEOUT)).initialize(any()); + verify(newProvider, timeout(TIMEOUT)).initialize(any(), isNull()); } @Test @@ -193,7 +194,7 @@ class NamedProvider { @DisplayName("should immediately return when calling the provider mutator") void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { FeatureProvider newProvider = createMockedProvider(); - doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any()); + doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any(), eq(DOMAIN_NAME)); Future providerMutation = executorService.submit(() -> providerRepository.setProvider( DOMAIN_NAME, @@ -357,7 +358,7 @@ void shouldHandleShutdownDuringProviderInitialization() throws Exception { FeatureProvider slowInitProvider = createMockedProvider(); AtomicBoolean shutdownCalled = new AtomicBoolean(false); - doDelayResponse(Duration.ofMillis(500)).when(slowInitProvider).initialize(any()); + doDelayResponse(Duration.ofMillis(500)).when(slowInitProvider).initialize(any(), isNull()); doAnswer(invocation -> { shutdownCalled.set(true); @@ -456,7 +457,7 @@ void shouldFallBackToDirectShutdownWhenExecutorRejectsTasks() throws Exception { return null; }) .when(oldProvider) - .initialize(any()); + .initialize(any(), isNull()); // Start async initialization (will block) providerRepository.setProvider( diff --git a/src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java b/src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java index f22a0811a..68f471b1e 100644 --- a/src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java +++ b/src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.spy; @@ -94,10 +95,10 @@ private void setupMockProvider(ErrorCode errorCode, String errorMessage, Provide while (true) {} }) .when(mockProvider) - .initialize(any()); + .initialize(any(), eq(providerState.name())); break; case FATAL: - doThrow(new FatalError(errorMessage)).when(mockProvider).initialize(any()); + doThrow(new FatalError(errorMessage)).when(mockProvider).initialize(any(), eq(providerState.name())); break; } // Configure all evaluation methods with a single helper diff --git a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java index b9c6bc159..e7f5e3e71 100644 --- a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java +++ b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java @@ -1,19 +1,15 @@ package dev.openfeature.sdk.fixtures; -import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doBlock; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doAnswer; +import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import dev.openfeature.sdk.FeatureProvider; -import dev.openfeature.sdk.ImmutableContext; import dev.openfeature.sdk.ProviderState; import java.io.FileNotFoundException; -import java.util.concurrent.CountDownLatch; import lombok.experimental.UtilityClass; -import org.mockito.stubbing.Answer; @UtilityClass public class ProviderFixture { @@ -33,33 +29,7 @@ public static FeatureProvider createMockedReadyProvider() { public static FeatureProvider createMockedErrorProvider() throws Exception { FeatureProvider provider = mock(FeatureProvider.class); doReturn(ProviderState.NOT_READY).when(provider).getState(); - doThrow(FileNotFoundException.class).when(provider).initialize(any()); - return provider; - } - - public static FeatureProvider createBlockedProvider(CountDownLatch latch, Runnable onAnswer) throws Exception { - FeatureProvider provider = createMockedProvider(); - doBlock(latch, createAnswerExecutingCode(onAnswer)).when(provider).initialize(new ImmutableContext()); - doReturn("blockedProvider").when(provider).toString(); - return provider; - } - - private static Answer createAnswerExecutingCode(Runnable onAnswer) { - return invocation -> { - onAnswer.run(); - return null; - }; - } - - public static FeatureProvider createUnblockingProvider(CountDownLatch latch) throws Exception { - FeatureProvider provider = createMockedProvider(); - doAnswer(invocation -> { - latch.countDown(); - return null; - }) - .when(provider) - .initialize(new ImmutableContext()); - doReturn("unblockingProvider").when(provider).toString(); + doThrow(FileNotFoundException.class).when(provider).initialize(any(), nullable(String.class)); return provider; } } diff --git a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java index d69cd821f..4976d5276 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java @@ -5,8 +5,13 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import dev.openfeature.sdk.ErrorCode; @@ -15,8 +20,10 @@ import dev.openfeature.sdk.Metadata; import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.ProviderState; import dev.openfeature.sdk.Value; import dev.openfeature.sdk.exceptions.GeneralError; +import dev.openfeature.sdk.providers.memory.InMemoryProvider; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -52,10 +59,63 @@ void shouldInitializeSuccessfully() { assertEquals("multiprovider", multiProvider.getMetadata().getName()); } + @SneakyThrows + @Test + void singleArgInitializeForwardsNullDomainToChildren() { + List providers = new ArrayList<>(2); + providers.add(mockProvider1); + providers.add(mockProvider2); + MultiProvider multiProvider = new MultiProvider(providers); + + multiProvider.initialize(null); + + verify(mockProvider1).initialize(isNull(), isNull()); + verify(mockProvider2).initialize(isNull(), isNull()); + } + + @SneakyThrows + @Test + void forwardsDomainToChildProviderInitialize() { + List providers = new ArrayList<>(2); + providers.add(mockProvider1); + providers.add(mockProvider2); + MultiProvider multiProvider = new MultiProvider(providers); + EvaluationContext context = new MutableContext().add("targetingKey", "user"); + + multiProvider.initialize(context, "my-domain"); + + verify(mockProvider1).initialize(same(context), eq("my-domain")); + verify(mockProvider2).initialize(same(context), eq("my-domain")); + } + + @SneakyThrows + @Test + void forwardsDomainToLegacySingleArgChildProvidersWithoutError() { + InMemoryProvider legacyProvider1 = new InMemoryProvider(Map.of()) { + @Override + public Metadata getMetadata() { + return () -> "legacy-provider-1"; + } + }; + InMemoryProvider legacyProvider2 = new InMemoryProvider(Map.of()) { + @Override + public Metadata getMetadata() { + return () -> "legacy-provider-2"; + } + }; + MultiProvider multiProvider = new MultiProvider(List.of(legacyProvider1, legacyProvider2)); + EvaluationContext context = new MutableContext().add("targetingKey", "user"); + + multiProvider.initialize(context, "my-domain"); + + assertEquals(ProviderState.READY, legacyProvider1.getState()); + assertEquals(ProviderState.READY, legacyProvider2.getState()); + } + @SneakyThrows @Test void shouldHandleInitializationFailure() { - doThrow(new GeneralError()).when(mockProvider1).initialize(any()); + doThrow(new GeneralError()).when(mockProvider1).initialize(any(), isNull()); doThrow(new GeneralError()).when(mockProvider1).shutdown(); List providers = new ArrayList<>(2); providers.add(mockProvider1); @@ -66,6 +126,18 @@ void shouldHandleInitializationFailure() { assertDoesNotThrow(multiProvider::shutdown); } + @SneakyThrows + @Test + void continuesWhenShutdownFailsAfterInitializeFailure() { + doThrow(new GeneralError()).when(mockProvider1).initialize(any(), isNull()); + MultiProvider multiProvider = spy(new MultiProvider(List.of(mockProvider1))); + doThrow(new RuntimeException("shutdown failed")).when(multiProvider).shutdown(); + + assertThrows(ExecutionException.class, () -> multiProvider.initialize(null)); + + verify(multiProvider).shutdown(); + } + @Test void shouldHandleDuplicateProviderNames() { when(mockProvider1.getMetadata()).thenReturn(() -> "provider"); diff --git a/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java b/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java index 2d88f698f..97395a5bd 100644 --- a/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java +++ b/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -32,7 +33,7 @@ private FeatureProvider createMockedProvider(String name, AtomicInteger shutdown }) .when(provider) .shutdown(); - doAnswer(invocation -> null).when(provider).initialize(any()); + doAnswer(invocation -> null).when(provider).initialize(any(), isNull()); return provider; }