From 87f226ed458d0a769c004017ddfc50759d478ad5 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Thu, 2 Jul 2026 16:47:04 -0400 Subject: [PATCH 01/10] feat: supply bound domain to provider initialization Signed-off-by: Jonathan Norris --- .gitignore | 2 + .../dev/openfeature/sdk/FeatureProvider.java | 33 ++++ .../sdk/FeatureProviderStateManager.java | 7 +- .../openfeature/sdk/ProviderRepository.java | 35 +++- .../sdk/multiprovider/MultiProvider.java | 16 +- .../sdk/DomainScopedProviderSpecTest.java | 171 ++++++++++++++++++ .../sdk/FeatureProviderStateManagerTest.java | 15 ++ .../sdk/InitializeBehaviorSpecTest.java | 46 ++++- .../openfeature/sdk/OpenFeatureAPITest.java | 3 +- .../sdk/ProviderRepositoryTest.java | 23 +-- .../sdk/e2e/steps/ProviderSteps.java | 4 +- .../sdk/fixtures/ProviderFixture.java | 7 +- .../sdk/multiprovider/MultiProviderTest.java | 3 +- .../sdk/vmlens/ProviderRepositoryCT.java | 2 +- 14 files changed, 334 insertions(+), 33 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java diff --git a/.gitignore b/.gitignore index bd26102a2..2d842bb5c 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,5 @@ target # used for spec compliance tooling java-report.json +.worktrees/ +.worktrees/ diff --git a/src/main/java/dev/openfeature/sdk/FeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProvider.java index 22819ef10..1a6a63490 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProvider.java @@ -1,5 +1,6 @@ package dev.openfeature.sdk; +import edu.umd.cs.findbugs.annotations.Nullable; import java.util.ArrayList; import java.util.List; @@ -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..b0eb29003 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java @@ -1,6 +1,7 @@ package dev.openfeature.sdk; import dev.openfeature.sdk.exceptions.OpenFeatureError; +import edu.umd.cs.findbugs.annotations.Nullable; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import lombok.extern.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..924d38291 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -3,6 +3,7 @@ import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.exceptions.OpenFeatureError; import dev.openfeature.sdk.internal.ConfigurableThreadFactory; +import edu.umd.cs.findbugs.annotations.Nullable; import java.util.List; import java.util.Map; import java.util.Optional; @@ -154,7 +155,7 @@ public void setProvider( } private void prepareAndInitializeProvider( - String domain, + @Nullable String domain, FeatureProvider newProvider, Consumer afterSet, Consumer afterInit, @@ -168,6 +169,7 @@ private void prepareAndInitializeProvider( if (isShuttingDown.get()) { throw new IllegalStateException("Provider cannot be set while repository is shutting down"); } + validateDomainScopedBinding(domain, newProvider); FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider); if (existing == null) { openFeatureAPI.registerGlobalProvider(newProvider); @@ -185,15 +187,39 @@ 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; + } + FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider); + if (existing == null) { + return; + } + boolean currentlyDefault = isDefaultProvider(newProvider); + List boundNamedDomains = getDomainsForProvider(newProvider); + 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 FeatureProviderStateManager getExistingStateManagerForProvider(FeatureProvider provider) { for (FeatureProviderStateManager stateManager : stateManagers.values()) { if (stateManager.hasSameProvider(provider)) { @@ -208,6 +234,7 @@ private FeatureProviderStateManager getExistingStateManagerForProvider(FeaturePr } private void initializeProvider( + @Nullable String domain, FeatureProviderStateManager newManager, Consumer afterInit, Consumer afterShutdown, @@ -215,7 +242,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..bb0242fad 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java @@ -6,6 +6,7 @@ import dev.openfeature.sdk.Metadata; import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.Value; +import edu.umd.cs.findbugs.annotations.Nullable; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.ArrayList; import java.util.Collection; @@ -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, null); 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..e701dfd56 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java @@ -0,0 +1,171 @@ +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.verify; + +import java.time.Duration; +import java.util.concurrent.atomic.AtomicReference; +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()); + } + + @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 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(); + + assertThatCode(() -> { + api.setProvider(DOMAIN_A, provider); + api.setProvider(DOMAIN_B, provider); + }) + .doesNotThrowAnyException(); + + verify(provider, timeout(1000)).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 final class DomainScopedTestProvider extends EventProvider { + + private final AtomicReference initDomain = new AtomicReference<>(); + + @Override + public Metadata getMetadata() { + return () -> "domain-scoped-test"; + } + + @Override + public boolean isDomainScoped() { + return true; + } + + @Override + public void initialize(EvaluationContext evaluationContext, String domain) { + 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(); + } + } +} 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/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index c9d69ad73..519aebde2 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(), any()); 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(), any()); 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(), any()); 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(), any()); 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(), any()); doAnswer(invocation -> { shutdownCalled.set(true); @@ -456,7 +457,7 @@ void shouldFallBackToDirectShutdownWhenExecutorRejectsTasks() throws Exception { return null; }) .when(oldProvider) - .initialize(any()); + .initialize(any(), any()); // 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..a43c69d18 100644 --- a/src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java +++ b/src/test/java/dev/openfeature/sdk/e2e/steps/ProviderSteps.java @@ -94,10 +94,10 @@ private void setupMockProvider(ErrorCode errorCode, String errorMessage, Provide while (true) {} }) .when(mockProvider) - .initialize(any()); + .initialize(any(), any()); break; case FATAL: - doThrow(new FatalError(errorMessage)).when(mockProvider).initialize(any()); + doThrow(new FatalError(errorMessage)).when(mockProvider).initialize(any(), any()); 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..2548313f6 100644 --- a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java +++ b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java @@ -8,7 +8,6 @@ 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; @@ -33,13 +32,13 @@ 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()); + doThrow(FileNotFoundException.class).when(provider).initialize(any(), 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()); + doBlock(latch, createAnswerExecutingCode(onAnswer)).when(provider).initialize(any(), any()); doReturn("blockedProvider").when(provider).toString(); return provider; } @@ -58,7 +57,7 @@ public static FeatureProvider createUnblockingProvider(CountDownLatch latch) thr return null; }) .when(provider) - .initialize(new ImmutableContext()); + .initialize(any(), any()); doReturn("unblockingProvider").when(provider).toString(); 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..6d54bebf9 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java @@ -5,6 +5,7 @@ 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.isNull; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -55,7 +56,7 @@ void shouldInitializeSuccessfully() { @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); diff --git a/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java b/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java index 2d88f698f..4e371e3e3 100644 --- a/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java +++ b/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java @@ -32,7 +32,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(), any()); return provider; } From f6854acfd0dbcbed7332da585f925981d14eb073 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 10:18:58 -0400 Subject: [PATCH 02/10] refactor: use javax.annotation.Nullable for domain parameter Signed-off-by: Jonathan Norris --- src/main/java/dev/openfeature/sdk/FeatureProvider.java | 2 +- .../java/dev/openfeature/sdk/FeatureProviderStateManager.java | 2 +- src/main/java/dev/openfeature/sdk/ProviderRepository.java | 2 +- .../java/dev/openfeature/sdk/multiprovider/MultiProvider.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/FeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProvider.java index 1a6a63490..902c0d7d1 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProvider.java @@ -1,8 +1,8 @@ package dev.openfeature.sdk; -import edu.umd.cs.findbugs.annotations.Nullable; import java.util.ArrayList; import java.util.List; +import javax.annotation.Nullable; /** * The interface implemented by upstream flag providers to resolve flags for diff --git a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java index b0eb29003..4cfda2c45 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java @@ -1,9 +1,9 @@ package dev.openfeature.sdk; import dev.openfeature.sdk.exceptions.OpenFeatureError; -import edu.umd.cs.findbugs.annotations.Nullable; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.Nullable; import lombok.extern.slf4j.Slf4j; @Slf4j diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 924d38291..917bf07a0 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -3,7 +3,6 @@ import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.exceptions.OpenFeatureError; import dev.openfeature.sdk.internal.ConfigurableThreadFactory; -import edu.umd.cs.findbugs.annotations.Nullable; import java.util.List; import java.util.Map; import java.util.Optional; @@ -18,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 diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java index bb0242fad..cd76f43bd 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java @@ -6,7 +6,6 @@ import dev.openfeature.sdk.Metadata; import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.Value; -import edu.umd.cs.findbugs.annotations.Nullable; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.ArrayList; import java.util.Collection; @@ -20,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; From 2a8d3692d2ac12018a23cb906346ba37975d4595 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 10:27:42 -0400 Subject: [PATCH 03/10] test: cover initialize default method backward compatibility Signed-off-by: Jonathan Norris --- ...erInitializeBackwardCompatibilityTest.java | 273 ++++++++++++++++++ 1 file changed, 273 insertions(+) create mode 100644 src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java 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..c3ec79f6b --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java @@ -0,0 +1,273 @@ +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("two-arg default delegates to a single-arg override") + void twoArgDefaultDelegatesToSingleArgOverride() throws Exception { + AtomicInteger singleArgInitCount = new AtomicInteger(); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + + provider.initialize(new ImmutableContext(), DOMAIN); + + assertThat(singleArgInitCount).hasValue(1); + } + + @Test + @DisplayName("two-arg override is used without invoking single-arg") + void twoArgOverrideDoesNotInvokeSingleArg() throws Exception { + AtomicInteger singleArgInitCount = new AtomicInteger(); + AtomicReference domainReceived = new AtomicReference<>(); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext, String domain) { + domainReceived.set(domain); + } + + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + + provider.initialize(new ImmutableContext(), DOMAIN); + + assertThat(domainReceived).hasValue(DOMAIN); + assertThat(singleArgInitCount).hasValue(0); + } + + @Test + @DisplayName("two-arg-only override receives null domain for default binding") + void twoArgOnlyOverrideReceivesNullDomain() throws Exception { + AtomicInteger singleArgInitCount = new AtomicInteger(); + AtomicReference domainReceived = new AtomicReference<>("unset"); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext, String domain) { + domainReceived.set(domain); + } + + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + + provider.initialize(new ImmutableContext(), null); + + assertThat(domainReceived.get()).isNull(); + assertThat(singleArgInitCount).hasValue(0); + } + + @Test + @DisplayName("single-arg override is only invoked once per initialization") + void singleArgOverrideIsOnlyInvokedOnce() throws Exception { + AtomicInteger singleArgInitCount = new AtomicInteger(); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + + provider.initialize(new ImmutableContext(), DOMAIN); + provider.initialize(new ImmutableContext(), DOMAIN); + + assertThat(singleArgInitCount).hasValue(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() { + AtomicInteger singleArgInitCount = new AtomicInteger(); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + + api.setProvider(DOMAIN, provider); + + await().atMost(Duration.ofSeconds(5)) + .untilAsserted(() -> assertThat(singleArgInitCount).hasValue(1)); + } + + @Test + @DisplayName("legacy single-arg provider is initialized when registered as the default provider") + void legacySingleArgProviderInitializedAsDefault() { + AtomicInteger singleArgInitCount = new AtomicInteger(); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + + api.setProvider(provider); + + await().atMost(Duration.ofSeconds(5)) + .untilAsserted(() -> assertThat(singleArgInitCount).hasValue(1)); + } + + @Test + @DisplayName("two-arg-only provider receives the bound domain from the SDK") + void twoArgOnlyProviderReceivesBoundDomainFromSdk() { + AtomicInteger singleArgInitCount = new AtomicInteger(); + AtomicReference domainReceived = new AtomicReference<>(); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext, String domain) { + domainReceived.set(domain); + } + + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + + api.setProvider(DOMAIN, provider); + + await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { + assertThat(domainReceived).hasValue(DOMAIN); + assertThat(singleArgInitCount).hasValue(0); + }); + } + + @Test + @DisplayName("two-arg-only default provider receives null domain from the SDK") + void twoArgOnlyDefaultProviderReceivesNullDomainFromSdk() { + AtomicInteger singleArgInitCount = new AtomicInteger(); + AtomicReference domainReceived = new AtomicReference<>("unset"); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext, String domain) { + domainReceived.set(domain); + } + + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + + api.setProvider(provider); + + await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { + assertThat(domainReceived.get()).isNull(); + assertThat(singleArgInitCount).hasValue(0); + }); + } + } + + @Nested + @DisplayName("FeatureProviderStateManager") + class StateManager { + + @Test + @DisplayName("delegates two-arg initialize to a legacy single-arg provider") + void delegatesTwoArgInitializeToLegacySingleArgProvider() throws Exception { + AtomicInteger singleArgInitCount = new AtomicInteger(); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + FeatureProviderStateManager stateManager = new FeatureProviderStateManager(provider); + + stateManager.initialize(new ImmutableContext(), DOMAIN); + + assertThat(singleArgInitCount).hasValue(1); + } + + @Test + @DisplayName("only invokes two-arg initialize once for a legacy single-arg provider") + void onlyInvokesLegacySingleArgProviderOncePerStateManagerInit() throws Exception { + AtomicInteger singleArgInitCount = new AtomicInteger(); + FeatureProvider provider = new TestProvider() { + @Override + public void initialize(EvaluationContext evaluationContext) { + singleArgInitCount.incrementAndGet(); + } + }; + FeatureProviderStateManager stateManager = new FeatureProviderStateManager(provider); + + stateManager.initialize(new ImmutableContext(), DOMAIN); + stateManager.initialize(new ImmutableContext(), DOMAIN); + + assertThat(singleArgInitCount).hasValue(1); + } + } + + private abstract static class TestProvider 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(); + } + } +} From 4e73b990175a0d0a8f5eccfdc33f3c3acd236c47 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 10:29:12 -0400 Subject: [PATCH 04/10] test: tighten ProviderFixture initialize mock matchers Signed-off-by: Jonathan Norris --- .../dev/openfeature/sdk/fixtures/ProviderFixture.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java index 2548313f6..60ebedf73 100644 --- a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java +++ b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java @@ -1,13 +1,14 @@ package dev.openfeature.sdk.fixtures; import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doBlock; -import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doAnswer; 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; @@ -32,13 +33,15 @@ 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(), any()); + doThrow(FileNotFoundException.class).when(provider).initialize(new ImmutableContext(), nullable(String.class)); return provider; } public static FeatureProvider createBlockedProvider(CountDownLatch latch, Runnable onAnswer) throws Exception { FeatureProvider provider = createMockedProvider(); - doBlock(latch, createAnswerExecutingCode(onAnswer)).when(provider).initialize(any(), any()); + doBlock(latch, createAnswerExecutingCode(onAnswer)) + .when(provider) + .initialize(new ImmutableContext(), nullable(String.class)); doReturn("blockedProvider").when(provider).toString(); return provider; } @@ -57,7 +60,7 @@ public static FeatureProvider createUnblockingProvider(CountDownLatch latch) thr return null; }) .when(provider) - .initialize(any(), any()); + .initialize(new ImmutableContext(), nullable(String.class)); doReturn("unblockingProvider").when(provider).toString(); return provider; } From cc1cec22292e15f85ac85599ecb689c1c82461f7 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 10:35:15 -0400 Subject: [PATCH 05/10] test: align initialize mock matchers across PR test updates Signed-off-by: Jonathan Norris --- .../dev/openfeature/sdk/ProviderRepositoryTest.java | 12 ++++++------ .../dev/openfeature/sdk/e2e/steps/ProviderSteps.java | 5 +++-- .../openfeature/sdk/fixtures/ProviderFixture.java | 3 ++- .../openfeature/sdk/vmlens/ProviderRepositoryCT.java | 3 ++- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index 519aebde2..a4ca798f5 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -65,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(any(), any()); + doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(any(), isNull()); await().alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) @@ -122,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(), any()); + doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(any(), eq("a domain")); await().alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) @@ -153,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(), any()); + doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any(), isNull()); await().alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) @@ -194,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(), any()); + doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any(), eq(DOMAIN_NAME)); Future providerMutation = executorService.submit(() -> providerRepository.setProvider( DOMAIN_NAME, @@ -358,7 +358,7 @@ void shouldHandleShutdownDuringProviderInitialization() throws Exception { FeatureProvider slowInitProvider = createMockedProvider(); AtomicBoolean shutdownCalled = new AtomicBoolean(false); - doDelayResponse(Duration.ofMillis(500)).when(slowInitProvider).initialize(any(), any()); + doDelayResponse(Duration.ofMillis(500)).when(slowInitProvider).initialize(any(), isNull()); doAnswer(invocation -> { shutdownCalled.set(true); @@ -457,7 +457,7 @@ void shouldFallBackToDirectShutdownWhenExecutorRejectsTasks() throws Exception { return null; }) .when(oldProvider) - .initialize(any(), 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 a43c69d18..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(), any()); + .initialize(any(), eq(providerState.name())); break; case FATAL: - doThrow(new FatalError(errorMessage)).when(mockProvider).initialize(any(), 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 60ebedf73..64fe22c4a 100644 --- a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java +++ b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java @@ -1,6 +1,7 @@ package dev.openfeature.sdk.fixtures; import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doBlock; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; @@ -33,7 +34,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(new ImmutableContext(), nullable(String.class)); + doThrow(FileNotFoundException.class).when(provider).initialize(any(), nullable(String.class)); return provider; } diff --git a/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java b/src/test/java/dev/openfeature/sdk/vmlens/ProviderRepositoryCT.java index 4e371e3e3..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(), any()); + doAnswer(invocation -> null).when(provider).initialize(any(), isNull()); return provider; } From 1fc14b65249d63356652bab6d90509dea2f59cba Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 10:54:51 -0400 Subject: [PATCH 06/10] fix: address review feedback and forward domain in MultiProvider Signed-off-by: Jonathan Norris --- .gitignore | 2 - .../openfeature/sdk/ProviderRepository.java | 6 +- .../sdk/multiprovider/MultiProvider.java | 2 +- ...erInitializeBackwardCompatibilityTest.java | 184 +++++++----------- .../sdk/fixtures/ProviderFixture.java | 33 ---- .../sdk/multiprovider/MultiProviderTest.java | 44 +++++ 6 files changed, 121 insertions(+), 150 deletions(-) diff --git a/.gitignore b/.gitignore index 2d842bb5c..bd26102a2 100644 --- a/.gitignore +++ b/.gitignore @@ -15,5 +15,3 @@ target # used for spec compliance tooling java-report.json -.worktrees/ -.worktrees/ diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 917bf07a0..512fb455c 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -169,8 +169,8 @@ private void prepareAndInitializeProvider( if (isShuttingDown.get()) { throw new IllegalStateException("Provider cannot be set while repository is shutting down"); } - validateDomainScopedBinding(domain, newProvider); FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider); + validateDomainScopedBinding(domain, newProvider, existing); if (existing == null) { openFeatureAPI.registerGlobalProvider(newProvider); newStateManager = new FeatureProviderStateManager(newProvider); @@ -196,11 +196,11 @@ private void prepareAndInitializeProvider( } } - private void validateDomainScopedBinding(@Nullable String domain, FeatureProvider newProvider) { + private void validateDomainScopedBinding( + @Nullable String domain, FeatureProvider newProvider, @Nullable FeatureProviderStateManager existing) { if (!newProvider.isDomainScoped()) { return; } - FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider); if (existing == null) { return; } diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java index cd76f43bd..f1389b67e 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java @@ -112,7 +112,7 @@ public void initialize(EvaluationContext evaluationContext, @Nullable String dom Collection> tasks = new ArrayList<>(providers.size()); for (FeatureProvider provider : providers.values()) { tasks.add(() -> { - provider.initialize(evaluationContext, null); + provider.initialize(evaluationContext, domain); return null; }); Metadata providerMetadata = provider.getMetadata(); diff --git a/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java b/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java index c3ec79f6b..2a6303dd1 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java @@ -22,80 +22,44 @@ class DefaultMethodDelegation { @Test @DisplayName("two-arg default delegates to a single-arg override") void twoArgDefaultDelegatesToSingleArgOverride() throws Exception { - AtomicInteger singleArgInitCount = new AtomicInteger(); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); provider.initialize(new ImmutableContext(), DOMAIN); - assertThat(singleArgInitCount).hasValue(1); + assertThat(provider.singleArgInitCount()).isOne(); } @Test @DisplayName("two-arg override is used without invoking single-arg") void twoArgOverrideDoesNotInvokeSingleArg() throws Exception { - AtomicInteger singleArgInitCount = new AtomicInteger(); - AtomicReference domainReceived = new AtomicReference<>(); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext, String domain) { - domainReceived.set(domain); - } - - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + TwoArgInitProvider provider = new TwoArgInitProvider(); provider.initialize(new ImmutableContext(), DOMAIN); - assertThat(domainReceived).hasValue(DOMAIN); - assertThat(singleArgInitCount).hasValue(0); + 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 { - AtomicInteger singleArgInitCount = new AtomicInteger(); - AtomicReference domainReceived = new AtomicReference<>("unset"); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext, String domain) { - domainReceived.set(domain); - } - - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + TwoArgInitProvider provider = new TwoArgInitProvider(); provider.initialize(new ImmutableContext(), null); - assertThat(domainReceived.get()).isNull(); - assertThat(singleArgInitCount).hasValue(0); + assertThat(provider.initDomain()).isNull(); + assertThat(provider.singleArgInitCount()).isZero(); } @Test @DisplayName("single-arg override is only invoked once per initialization") void singleArgOverrideIsOnlyInvokedOnce() throws Exception { - AtomicInteger singleArgInitCount = new AtomicInteger(); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); provider.initialize(new ImmutableContext(), DOMAIN); provider.initialize(new ImmutableContext(), DOMAIN); - assertThat(singleArgInitCount).hasValue(2); + assertThat(provider.singleArgInitCount()).isEqualTo(2); } } @@ -114,84 +78,48 @@ void setupTest() { @Test @DisplayName("legacy single-arg provider is initialized when registered to a named domain") void legacySingleArgProviderInitializedForNamedDomain() { - AtomicInteger singleArgInitCount = new AtomicInteger(); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); api.setProvider(DOMAIN, provider); - await().atMost(Duration.ofSeconds(5)) - .untilAsserted(() -> assertThat(singleArgInitCount).hasValue(1)); + 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() { - AtomicInteger singleArgInitCount = new AtomicInteger(); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); api.setProvider(provider); - await().atMost(Duration.ofSeconds(5)) - .untilAsserted(() -> assertThat(singleArgInitCount).hasValue(1)); + 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() { - AtomicInteger singleArgInitCount = new AtomicInteger(); - AtomicReference domainReceived = new AtomicReference<>(); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext, String domain) { - domainReceived.set(domain); - } - - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + TwoArgInitProvider provider = new TwoArgInitProvider(); api.setProvider(DOMAIN, provider); await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { - assertThat(domainReceived).hasValue(DOMAIN); - assertThat(singleArgInitCount).hasValue(0); + assertThat(provider.initDomain()).isEqualTo(DOMAIN); + assertThat(provider.singleArgInitCount()).isZero(); }); } @Test @DisplayName("two-arg-only default provider receives null domain from the SDK") void twoArgOnlyDefaultProviderReceivesNullDomainFromSdk() { - AtomicInteger singleArgInitCount = new AtomicInteger(); - AtomicReference domainReceived = new AtomicReference<>("unset"); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext, String domain) { - domainReceived.set(domain); - } - - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + TwoArgInitProvider provider = new TwoArgInitProvider(); api.setProvider(provider); await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { - assertThat(domainReceived.get()).isNull(); - assertThat(singleArgInitCount).hasValue(0); + assertThat(provider.initDomain()).isNull(); + assertThat(provider.singleArgInitCount()).isZero(); }); } } @@ -203,40 +131,74 @@ class StateManager { @Test @DisplayName("delegates two-arg initialize to a legacy single-arg provider") void delegatesTwoArgInitializeToLegacySingleArgProvider() throws Exception { - AtomicInteger singleArgInitCount = new AtomicInteger(); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); FeatureProviderStateManager stateManager = new FeatureProviderStateManager(provider); stateManager.initialize(new ImmutableContext(), DOMAIN); - assertThat(singleArgInitCount).hasValue(1); + assertThat(provider.singleArgInitCount()).isOne(); } @Test @DisplayName("only invokes two-arg initialize once for a legacy single-arg provider") void onlyInvokesLegacySingleArgProviderOncePerStateManagerInit() throws Exception { - AtomicInteger singleArgInitCount = new AtomicInteger(); - FeatureProvider provider = new TestProvider() { - @Override - public void initialize(EvaluationContext evaluationContext) { - singleArgInitCount.incrementAndGet(); - } - }; + LegacySingleArgInitProvider provider = new LegacySingleArgInitProvider(); FeatureProviderStateManager stateManager = new FeatureProviderStateManager(provider); stateManager.initialize(new ImmutableContext(), DOMAIN); stateManager.initialize(new ImmutableContext(), DOMAIN); - assertThat(singleArgInitCount).hasValue(1); + assertThat(provider.singleArgInitCount()).isOne(); } } - private abstract static class TestProvider extends EventProvider { + /** + * 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() { diff --git a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java index 64fe22c4a..e7f5e3e71 100644 --- a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java +++ b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java @@ -1,20 +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.ArgumentMatchers.nullable; -import static org.mockito.Mockito.doAnswer; 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 { @@ -37,32 +32,4 @@ public static FeatureProvider createMockedErrorProvider() throws Exception { doThrow(FileNotFoundException.class).when(provider).initialize(any(), nullable(String.class)); return provider; } - - public static FeatureProvider createBlockedProvider(CountDownLatch latch, Runnable onAnswer) throws Exception { - FeatureProvider provider = createMockedProvider(); - doBlock(latch, createAnswerExecutingCode(onAnswer)) - .when(provider) - .initialize(new ImmutableContext(), nullable(String.class)); - 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(), nullable(String.class)); - doReturn("unblockingProvider").when(provider).toString(); - 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 6d54bebf9..1bcf93602 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java @@ -5,9 +5,12 @@ 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.verify; import static org.mockito.Mockito.when; import dev.openfeature.sdk.ErrorCode; @@ -16,8 +19,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; @@ -53,6 +58,45 @@ void shouldInitializeSuccessfully() { assertEquals("multiprovider", multiProvider.getMetadata().getName()); } + @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() { From 4a23ee92714d2635551bf59755019c2b667659d4 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 11:07:38 -0400 Subject: [PATCH 07/10] test: stabilize shared-provider multi-domain binding assertion Signed-off-by: Jonathan Norris --- .../sdk/DomainScopedProviderSpecTest.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java index e701dfd56..9c51ebee8 100644 --- a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java @@ -9,6 +9,7 @@ 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; @@ -96,14 +97,16 @@ void allowsRebindingDomainScopedProviderToSameNamedDomain() throws Exception { 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(); - assertThatCode(() -> { - api.setProvider(DOMAIN_A, provider); - api.setProvider(DOMAIN_B, provider); - }) - .doesNotThrowAnyException(); + api.setProviderAndWait(DOMAIN_A, provider); + api.setProviderAndWait(DOMAIN_B, provider); - verify(provider, timeout(1000)).initialize(any(), eq(DOMAIN_A)); + assertThat(api.getProvider(DOMAIN_A)).isSameAs(provider); + assertThat(api.getProvider(DOMAIN_B)).isSameAs(provider); + verify(provider, times(1)).initialize(any(), eq(DOMAIN_A)); } @Specification( From 620d8111da30343969e913e9a4eddfd30294b2b5 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 11:35:40 -0400 Subject: [PATCH 08/10] test: improve coverage for domain-scoped provider binding paths Signed-off-by: Jonathan Norris --- .../sdk/DomainScopedProviderSpecTest.java | 31 +++++++++++++++++++ ...erInitializeBackwardCompatibilityTest.java | 6 ++++ .../sdk/multiprovider/MultiProviderTest.java | 27 ++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java index 9c51ebee8..651e7a909 100644 --- a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java @@ -13,6 +13,7 @@ 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.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -92,6 +93,30 @@ void allowsRebindingDomainScopedProviderToSameNamedDomain() throws Exception { 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 binding a non-domain-scoped provider to multiple domains") void allowsBindingNonDomainScopedProviderToMultipleDomains() throws Exception { @@ -128,6 +153,7 @@ void domainScopedProviderReceivesBoundDomainAtInitialization() { private static final class DomainScopedTestProvider extends EventProvider { private final AtomicReference initDomain = new AtomicReference<>(); + private final AtomicInteger initializeCount = new AtomicInteger(); @Override public Metadata getMetadata() { @@ -141,6 +167,7 @@ public boolean isDomainScoped() { @Override public void initialize(EvaluationContext evaluationContext, String domain) { + initializeCount.incrementAndGet(); initDomain.set(domain); } @@ -170,5 +197,9 @@ public ProviderEvaluation getDoubleEvaluation(String key, Double default public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { return ProviderEvaluation.builder().value(defaultValue).build(); } + + int initCount() { + return initializeCount.get(); + } } } diff --git a/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java b/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java index 2a6303dd1..35b4428d0 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderInitializeBackwardCompatibilityTest.java @@ -19,6 +19,12 @@ class ProviderInitializeBackwardCompatibilityTest { @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 { diff --git a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java index 1bcf93602..4976d5276 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java @@ -10,6 +10,7 @@ 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; @@ -58,6 +59,20 @@ 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() { @@ -111,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"); From 7791a428d3217ea0381e4a624312f706f4961ab0 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 14:26:25 -0400 Subject: [PATCH 09/10] fix: use instance identity for domain-scoped provider binding Signed-off-by: Jonathan Norris --- .../openfeature/sdk/ProviderRepository.java | 36 ++++++++++++---- .../sdk/DomainScopedProviderSpecTest.java | 41 ++++++++++++++++++- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 512fb455c..c94100008 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -170,7 +170,7 @@ private void prepareAndInitializeProvider( throw new IllegalStateException("Provider cannot be set while repository is shutting down"); } FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider); - validateDomainScopedBinding(domain, newProvider, existing); + validateDomainScopedBinding(domain, newProvider); if (existing == null) { openFeatureAPI.registerGlobalProvider(newProvider); newStateManager = new FeatureProviderStateManager(newProvider); @@ -196,16 +196,18 @@ private void prepareAndInitializeProvider( } } - private void validateDomainScopedBinding( - @Nullable String domain, FeatureProvider newProvider, @Nullable FeatureProviderStateManager existing) { + private void validateDomainScopedBinding(@Nullable String domain, FeatureProvider newProvider) { if (!newProvider.isDomainScoped()) { return; } - if (existing == null) { + + boolean currentlyDefault = isDefaultProviderInstance(newProvider); + List boundNamedDomains = getBoundDomainsForProviderInstance(newProvider); + + if (!currentlyDefault && boundNamedDomains.isEmpty()) { return; } - boolean currentlyDefault = isDefaultProvider(newProvider); - List boundNamedDomains = getDomainsForProvider(newProvider); + if (domain == null) { if (!currentlyDefault) { throw new IllegalArgumentException("Domain-scoped provider cannot be bound to more than one domain"); @@ -220,19 +222,37 @@ private void validateDomainScopedBinding( } } + 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, diff --git a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java index 651e7a909..c977429cc 100644 --- a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java @@ -117,6 +117,22 @@ void allowsRebindingSameDomainScopedInstanceAsDefault() throws Exception { 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 { @@ -150,7 +166,7 @@ void domainScopedProviderReceivesBoundDomainAtInitialization() { .untilAsserted(() -> assertThat(provider.initDomain.get()).isEqualTo(DOMAIN_A)); } - private static final class DomainScopedTestProvider extends EventProvider { + private static class DomainScopedTestProvider extends EventProvider { private final AtomicReference initDomain = new AtomicReference<>(); private final AtomicInteger initializeCount = new AtomicInteger(); @@ -202,4 +218,27 @@ 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(); + } + } } From 68fa5da98d37d9372aff09ddeeeea654912c8530 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 14:50:24 -0400 Subject: [PATCH 10/10] test: shutdown OpenFeatureAPI after each domain-scoped spec test Signed-off-by: Jonathan Norris --- .../dev/openfeature/sdk/DomainScopedProviderSpecTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java index c977429cc..1115ff4e9 100644 --- a/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/DomainScopedProviderSpecTest.java @@ -15,6 +15,7 @@ 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; @@ -31,6 +32,11 @@ void setupTest() { 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 "